-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
tools: use GitHub Squash and Merge feature when using CQ #40666
Conversation
Maybe the repository check in the workflow? node/.github/workflows/commit-queue.yml Line 21 in b5aa08d
|
I've tried removing this check, but there are missing repo secrets, I don't know what values I should use: https://github.com/nodejs/node-auto-test/runs/4059202773?check_suite_focus=true |
Try to replace |
b345942
to
e4605a2
Compare
Wouldn't the script be easier to read and write if we used the github CLI instead of calling the API manually? The CLI is installed on the runners by default |
Maybe, maybe not. For merging PRs, the CLI doesn't provide the options to do that (see cli/cli#4645) so we'd have to defer to |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The CQ has landed successfully nodejs/node-auto-test#43, including the |
@nodejs/actions |
Ohhh that's an interesting approach to purple merge vs red close a PR without resorting to force-push to the developer's branch. As for @targos wdyt about suggesting a |
SGTM, I'll look into it as soon as the script here is refactored to use |
Landed in 19839f8...80b8440 |
PR-URL: #40666 Refs: https://docs.github.com/en/rest/reference/pulls#merge-a-pull-request Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #40666 Refs: https://docs.github.com/en/rest/reference/pulls#merge-a-pull-request Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #40666 Refs: https://docs.github.com/en/rest/reference/pulls#merge-a-pull-request Reviewed-By: Mary Marchini <oss@mmarchini.me> Reviewed-By: Michaël Zasso <targos@protonmail.com>
GitHub REST API allows us to edit the commit message when using "Squash and Merge" feature, that would allow the CQ to "purple merge" PRs without needing to force-push to the user branch when the PR contains one commit max.
I've tested locally with nodejs/node-auto-test#37, I also tried to test it using the CQ in nodejs/node-auto-test#40 but the CQ is not working there. I think we should land this, and revert if it's broken.
Refs: https://docs.github.com/en/rest/reference/pulls#merge-a-pull-request