From 28a7a0b5cb8e474f77da5223d837ea1b7ea97f04 Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Fri, 31 Mar 2023 11:05:22 -0400 Subject: [PATCH] github: tweak how mergify rules handle aged PRs 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: https://github.com/Mergifyio/mergify/issues/5036 Signed-off-by: John Mulligan --- .github/mergify.yml | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/mergify.yml b/.github/mergify.yml index 3986a583..bc1b387a 100644 --- a/.github/mergify.yml +++ b/.github/mergify.yml @@ -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" + - "lebel=priority-review" - "author=@maintainers" - "#approved-reviews-by>=1" actions: @@ -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"