Skip to content

Commit

Permalink
github: tweak how mergify rules handle aged PRs
Browse files Browse the repository at this point in the history
Previously, the mergify rules tried to define a window of time where
contributions from maintainers would not sit indefinitely waiting for
two reviews (as the maintainer contributing the patch might normally be
the one reviewing these things). Instead, we added a short cut where a
single review on a maintainer's PR would be auto merged after 15 days.
Unfortunately, this approach didn't take into account the fact that
reacting to feedback and pushing new patches "resets" the 'updated-at'
timer.

This change adds a new mergify rule to label any non-draft PR that has
been sitting unchanged for the 15 day window. If this label is applied
to a PR from a maintainer and the PR has one approving review the auto
merge will be initiated. This has a nice side benefit that the label can
be applied to important bugfixes manually to accelerate them.
Non-maintainer contributions will not automerge because of the label but
it can be used to help reviewers decide what to look at first. This
label can also be removed manually in the case the submitter or reviewer
decides that, for example, an update to the PR was big enough to warrant
resetting the time window or requiring two reviews.

Idea based on the discussion in:
Mergifyio/mergify#5036

Signed-off-by: John Mulligan <[email protected]>
  • Loading branch information
phlogistonjohn committed Mar 31, 2023
1 parent 9d4e55e commit f5120aa
Showing 1 changed file with 19 additions and 3 deletions.
22 changes: 19 additions & 3 deletions .github/mergify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ pull_request_rules:
- or:
# Any contributor's PR can be automerged with 2 (or more) reviews.
- "#approved-reviews-by>=2"
# A maintainer's contribution only needs 1 review BUT we give a grace
# period over just two weeks for a 2nd reviewer to hopefully appear.
# A maintainer's contribution that has already aged long enough to
# earn the "priority-review" label can be merged immediately.
# The label can also be applied manually in case of an important
# bugfix, etc.
- and:
- "updated-at<15 days ago"
- "label=priority-review"
- "author=@maintainers"
- "#approved-reviews-by>=1"
actions:
Expand All @@ -77,3 +79,17 @@ pull_request_rules:
comment:
message: "This pull request now has conflicts with the target branch.
Please resolve these conflicts and force push the updated branch."
# Label PRs that have been sitting there unchanged, aging like a fine wine
#
# NOTE: the updated-at "counter" resets every time the PR is changed so
# reacting to a reviewer's feedback and fixing a typo (for example) will
# reset the counter. Thus we now apply a label once we hit the 15 day window
# so that we know that PR had, at some time, sat unchanged for that long.
- name: Label aged PRs
conditions:
- "updated-at<15 days ago"
- "-draft"
actions:
label:
add:
- "priority-review"

0 comments on commit f5120aa

Please sign in to comment.