- Notifications
You must be signed in to change notification settings - Fork 2k
Automating docker-node part of the official Node Docker images release process#1646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Pehesi97 commented Feb 18, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
SimenB commented Feb 22, 2022
@Pehesi97 just notice this, exciting! Feel free to ping if/when you're ready for feedback. /cc @nodejs/docker |
Co-authored-by: Simen Bekkhus <[email protected]>
SimenB commented Mar 10, 2022
I'll merge this later today (CET) unless anybody says otherwise - would like to see it handle the new v17 release |
SimenB commented Mar 10, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I've enabled EDIT: right
I'd love to hear those reasons 🙂 I've now enabled the auto merge feature, and we already have branch protection rules, so I think all 3 conditions in the docs are ok: https://github.com/peter-evans/enable-pull-request-automerge#conditions We also require a review, so we probably need the auto approve action as well |
Pehesi97 commented Mar 10, 2022
Yeah, I think that will work. My main issue when trying to make auto-merge work was that the check suite isn't static (we create it dynamically using GitHub Actions) and it wasn't enough to trigger the "Require status checks to pass before merging" rule. I didn't try requiring approvals for the auto-merge to work, and all the others were already satisfied by the time I opened a test PR. About the auto-approve-action, I think https://github.com/hmarr/auto-approve-action will do the trick. Do you want me to do the changes? |
SimenB commented Mar 10, 2022
If it works, I'd prefer to use auto merge over a manual poll, yes 🙂 |
Uh oh!
There was an error while loading. Please reload this page.
Pehesi97 commented Mar 10, 2022
@SimenB actually, I don't think that will work. For auto-approving the PR, we would need the checks to pass. That's kinda the point of polling the GitHub API. |
Co-authored-by: Simen Bekkhus <[email protected]>
SimenB commented Mar 10, 2022
Fair enough. Merging will fail unless an approval is added though, so we need that part anyways |
Uh oh!
There was an error while loading. Please reload this page.
SimenB left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of these actions can run with the default token.
However, approval must happen with a different token as it's not allowed to approve your own PR
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Simen Bekkhus <[email protected]>
Co-authored-by: Simen Bekkhus <[email protected]>
Co-authored-by: Simen Bekkhus <[email protected]>
Co-authored-by: Simen Bekkhus <[email protected]>
SimenB commented Mar 10, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@Pehesi97 see Pehesi97#27 (also, very sorry for all the back and forth 😬) |
Pehesi97 commented Mar 10, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
It's OK. I understand. |
SimenB commented Mar 10, 2022
I'm not 100% the automerge will work, but as mentioned in Pehesi97#27 (comment), just getting the PR is a great first step. Thank you so much for working on this @Pehesi97, and thanks for your patience! |
shaneog commented Mar 16, 2022
Should this have kicked off a new build/release process for this Node.js release from yesterday? Or should I open a PR for that? |
nschonni commented Mar 16, 2022
Looks like the Alpine builds still aren't available |
shaneog commented Mar 17, 2022
Just released: PR here |

Description
Motivation and Context
#1312
Testing Details
Example Output(if appropriate)
Types of changes
Checklist