cabal
cabal copied to clipboard
implement mergify rules for release branches
We only handled the case of backports previously, but the current release checklist expects that we can commit PRs to release branches during a release (e.g. changelogs, because we want the list of changelog.d files that are actually part of the release).
Template B: This PR does not modify behaviour or interface
E.g. the PR only touches documentation or tests, does refactorings, etc.
Include the following checklist in your PR:
- [x] Patches conform to the coding conventions.
- [ ] Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions). — It is, but Mergify only uses the config file on
master
Naturally, it turns out that Mergify has no way to say "doesn't match regex". I guess there was a reason we didn't have rules for it. (ETA: no, it's just really sensitive to how you write it)
The rules I implemented are a hybrid of the master and backport rules:
- 2 reviews required
- no delay between setting the merge label and merging
We should probably iron out what we want the actual rules to be.
And while we're thinking about merge rules, should we prevent merging PRs with a blocked: label?
And while we're thinking about merge rules, should we prevent merging PRs with a
blocked:label?
If that's not too fiddly, sounds good.
Waiting on #10136
I believe some documentation of our Mergify setup is hosted in CONTRIBUTING.md. Could it be updated to explain, in plain English, how this PR extends the setup? Maybe, at this point, and given recent talks about usefulness of Mergify (see the last meeting notes -- there's a great rundown of the Mergidy features we currently use), Mergify deserves a subsection in that document?.. Maybe a wiki page and a link to it from that doc would be fine, 'cause a casual contributor doesn't have to know all the nits and grits...
Casual contributors won't be involved with this. It's specifically for commits during releases that are made to the release branch and "backported" to master, such as the changelog updates. Without these rules, they must be committed manually because Mergify will ignore non-backport commits not to master. So if they need to be documented anywhere, it would be the release guide in the wiki.
Casual contributors won't be involved with this.
Yes, that's why I'm suggesting an alternative to shoving all the Mergify-related stuff into CONTRIBUTING.md and to create a wiki page instead. On the other hand, wiki pages end to bit rot...
So if they need to be documented anywhere, it would be the release guide in the wiki.
SGTM!
Re the meeting notes (https://hackmd.io/36ipih8STyyBV9kvSYi2Dw), I note that GitHub does not support the rules defined in this PR. It defines per-branch rules, so we would have to use one reviewer always on release branches (these rules specify two for non-backports, as with master). There is also no way to specify any delay with GitHub branch protection, from what I saw.
Additionally, Mergify's (but not GitHub's, AFAICT) "squash" method ensures the PR is mentioned in the commit message. @fgaz considered this important for changelog-d to work correctly.
Any other comments here, or should I add the label?
Fire it! :fire: :fire: :fire:
Hm, I think this will cause problems with #10260, I'm surprised it's not complaining about conflicts.
@mergify refresh
refresh