Enabling GitHub's "auto-merge" feature

[fingolfin]

Sometimes a PR has been reviewed and approved, and could be merged, but the CI is still running. Then it may be forgotten to merge the PR, and we only merge it hours or days later. Then we merge the PR, and BAM master fails -- because in the meantime work was done on master that broke the test results for the PR, but we didn't notice, as we didn't rerun the tests.

The GitHub "auto-merge" feature can help with that; I've now enabled it experimentally for Oscar.jl. For details on that, see here. So now when you look at a PR where the CI is still running, or is currently failing, instead of a button saying "Merge/Squash/Rebase PR" you see a button saying something like "Enable auto-merge (rebase)". It is then automatically merged once it satisfies all condition configured in the "branch protection rules" for master, which currently means:

• must have at least one approving review and no "changes requested"
• must pass a certain set of CI status checks, which currently is just test (1.6, x64, ubuntu-latest) but we can add more (but we'll want to exclude tests that regularly fail due to some reasons, e.g. as in issue Internal stackoverflow julia master + MacOS #842)

However, there can still be broken master branch builds: Sometimes it can happen that we merge two PRs immediately after each other. Even if each just passed their CI tests, the final result can fail due to unforeseen interactions between the changes in the two PRs. To deal with that, I look forward to using "Merge Queues" which basically allow to say "merge this PR, then run the CI, and if that fails, revert it" -- I've already applied for us to join the beta program.

CC @lkastner @benlorenz who previously expressed concerns about broken master builds.

[thofma]

Is this a question whether we should use it?

[fingolfin]

Yes and no. I've already enabled this feature, but it may or may not turn out to be useful; so I thought I should explain what happened to anyone who stumbled over it. Hence this post.

For myself, the experience so far has been mixed: on the one hand I was able to approve && "enable automerge" a few PRs before their CI tests were complete, which is convenient. On the downside, I was not able to immediately merge some of my more trivial PRs (such as PR #865) because now first someone else has to approve it. (Of course while it is inconvenient for me, it may be also viewed as a positive thing -- but only if enough people frequently look at new PRs and approve at least the "obvious" ones...)

[fingolfin]

I've disabled this again for now, as it turns out that it prevents "self-merges": sometimes you have a trivial change, maybe a typo; with "auto-merge" unfortunately someone else always must approve a PR before it can be merged/rebased/squashed. That was a bit too annoying (while in general I prefer a four-eye approach even for seemingly "trivial" patches, it doesn't work too well right now in practice)... I hope we'll get on the "Merge queue" beta list soon as a nicer replacement.

[fieker]

On Tue, Jan 04, 2022 at 03:15:12AM -0800, Max Horn wrote: I've disabled this again for now, as it turns out that it prevents "self-merges": sometimes you have a trivial change, maybe a typo; with "auto-merge" unfortunately someone else always must approve a PR before it can be merged/rebased/squashed. That was a bit too annoying (while in general I prefer a four-eye approach even for seemingly "trivial" patches, it doesn't work too well right now in practice)... I hope we'll get on the "Merge queue" beta list soon as a nicer replacement.

I still can't - self merge (force) - self review

…

-- Reply to this email directly or view it on GitHub: #845 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[fingolfin]

self-review was never possible (resp. it is possible, but only with the option "Comment", not Approve/Require Changes), and IMHO that makes perfect sense.

But as to "self merge": aha, we have "Require approvals" and "Include administrators" on. I've disabled the "Require approval" for now. Try again?