Should we enforce last pusher cannot approve in Mergify? #10625
oxarbitrage
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Today I rebased two fork PRs (#10570, #10590) to fix CHANGELOG conflicts and pushed to the fork branches. GitHub warned that my approval should not count since I was the last pusher — but Mergify merged them anyway because its queue rule only checks
#approved-reviews-by >= 1without enforcing that constraint.This means a maintainer who pushes to a fork branch can also approve and merge it, bypassing the intent of the "someone else should approve new changes" safeguard.
Options:
Add the rule to Mergify — require the approver to be different from the last pusher. This means fork rebases always need a second team member, even for trivial CHANGELOG conflicts.
Leave as-is — Mergify accepts any approval. The GitHub UI still warns, and we rely on maintainers to use judgment. For cases where the rebase is trivial and the review was already thorough, this avoids unnecessary round-trips.
Middle ground — enforce the rule only for PRs with specific labels (e.g.,
A-security) where the extra check matters most.Context: both PRs had deep local review (code, constants, tests, clippy, fmt) before the rebase. The push was purely mechanical (CHANGELOG conflict resolution). Requesting a second approval would have added latency for a rubber-stamp.
Thoughts?
All reactions