Enabled auto-merging in the scala/scala repo


Just a quick note for the maintainers/committers of the scala/scala repo: we’ve enabled the auto-merge support that GitHub has. This comes in handy for those moments when your PR is approved but you need to make a straightforward change before it’s merge (e.g. rebase to get a fresh CI status, resolve mima filter conflicts or squash commits).

Have a look at https://docs.github.com/en/github/administering-a-repository/managing-auto-merge-for-pull-requests-in-your-repository to see how it’s not automatic on all pull requests and how both CI status and requested reviews are blockers from it triggering.

Any questions, comments or concerns, let us know.

1 Like

what criteria are required before a merge?

1 Like

I assume you don’t mean the mechanical criteria for the auto-merging to happen, but about merging in general. “Same as before” is the cheeky, but still true, answer. I don’t think it’s ever been decided - I definitely can’t spot in the README.md or CONTRIBUTING.md with 5 minutes of looking. At this point I just have a feel for it. A balance between the risk of the change and who’s approved it and with what message. I try to convey my level of certainty with something from “Soft LGTM” to “Perfect!” :smile:. An approval by someone on the Lightbend Team is a good rule of thumb, with the exception of when we’re just before a release. @SethTisue, do you have some other pointers? Oh, and both CIs passing is a given (so don’t merge the PRs that I insta-approve without waiting for the test to even start running! :smile:)

I meant mechanical—an approving review is not currently required on scala/scala afaict

related to this, I think this github action might be useful to prevent accidentally merging fixup/squash commits, especially if it’s happening automatically: https://github.com/marketplace/actions/block-fixup-commit-merge

Yeah, it isn’t, as a setting of the repo. I’m a little concerned that setting that requirement might set the incorrect expectation that 1 approval is sufficient for all PRs which, in my eyes, isn’t always the case.

But, mechanically, any requested reviews are required to be satisfied before the auto-merge goes through.