In this case we don’t have to check the box, we can directly click to mean “merge when CI completes”.
We can still check the box to merge without waiting (or merge with a broken CI if we intend to).
This saves someone from polling the status of CI before clicking on the merge button.
What happens if the CI does’t pass? Or if the author updates the branch while CI is running? Does the “merge after CI pass” merges after an update, or does it get cancelled and someone needs to click on “merge” again?
+1, we’ve been using it in IREE for a few years now and it works well and without surprises.
What happens if the CI does’t pass? Or if the author updates the branch while CI is running?
IIRC, a new push will cancel the current round of CI and the automerge can happen when the new CI run finishes. The author or other people with commit access can cancel automerge (e.g., when you change your mind or spot a new blocking issue).
This is what I’m worried about. What if the fix is not the right one? New developers don’t always know how to fix a particular problem, and I use the merge button as a way to say “I have seen your fix and want to merge anyway”. Having to cancel the auto-merge may not be enough (for example, if I’m sleeping).
But if any fix would bypass that last check, than this is worrying.
This is what I’m worried about. What if the fix is not the right one? New developers don’t always know how to fix a particular problem, and I use the merge button as a way to say “I have seen your fix and want to merge anyway”. Having to cancel the auto-merge may not be enough (for example, if I’m sleeping).
How is this different from the current process? Right now, and after this setting is enabled, you start clicking the merge button as soon as you, the author, feel confident the change should land. The only difference is whether you need to manually wait for CI to pass before clicking the button or are able to do so prior to CI passing.
Exactly. Automerge is just ‘press the merge button for me when CI comes back green’. If something is merged prematurely for whatever reason and by whatever criteria, the same revert/post-commit review policies apply.
I’m assuming you are concerned about the case where a reviewer enables auto-merge for a PR by a contributor without commit access. In this case, per the GitHub docs:
After you enable auto-merge for a pull request, if someone who does not have write permissions to the repository pushes new changes to the head branch or switches the base branch of the pull request, auto-merge will be disabled.
Thanks for writing this up (I was hoping to do a write up yesterday and didn’t get to it).
I think most of the opposition to enabling this is a lack of communication around the safeguards/limited profile of what this actually does. Maybe partially due to my poor explanation on the other thread. It’s pretty limited in scope and essentially just replaces when someone would already be hitting the merge button in my opinion.
I voiced opposition in the other thread. I’d like to retract that opposition based on my now better understanding of what this feature does. This feature seems reasonable to enable, thanks!
+1 here. I’d like to suggest that we should update our docs to be super explicit about this being done after the patch is approved and ready to go in. The wording on the github docs make it seem like there are multiple workflows possible, and we should be explicit about which one is being allowed.