Merge immediately if branch is up to date and passes tests#397
Merge immediately if branch is up to date and passes tests#397dknopik merged 2 commits intosigp:stablefrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Mergify configuration to only queue PRs that are behind the target branch and to auto-merge up-to-date PRs once tests pass and reviews are approved.
- Restricts existing queue rule (
#commits-behind > 0) so it only enqueues outdated PRs - Adds a new rule to directly merge up-to-date PRs (
#commits-behind == 0) after successful tests and at least one approval
Comments suppressed due to low confidence (2)
.github/mergify.yml:85
- [nitpick] The rule name is very long and may be hard to scan; consider shortening it to something like
Queue outdated ready-for-merge PRsfor clarity.
- name: Add ready-to-merge labeled PRs that are not up to date with target to merge queue
.github/mergify.yml:95
- [nitpick] These two rules share several identical conditions (e.g.
base!=stable,label=ready-for-merge,label!=do-not-merge); consider extracting common conditions or using a reusable snippet/template to reduce duplication.
- name: Directly merge ready-to-merge labeled PRs that are up to date with target
.github/mergify.yml
Outdated
| type: APPROVE | ||
|
|
||
| - name: Add ready-to-merge labeled PRs to merge queue | ||
| - name: Add ready-to-merge labeled PRs that are not up to date with target to merge queue |
There was a problem hiding this comment.
Following up on the copilot comment, does this name appear somewhere? If so, maybe we can use only "Add to merge queue" and add the rest as a comment. Otherwise, looks good to me. There's some duplication, but not a big deal imo.
There was a problem hiding this comment.
Following up on the copilot comment, does this name appear somewhere?
I think it appears in small letters in the Github actions window below the PR.
I made the names shorter 👍
diegomrsantos
left a comment
There was a problem hiding this comment.
Thanks for addressing this
|
Need to point this to
Np, hopefully it makes it a bit more convenient to use :) |
|
Some required checks have failed. Could you please take a look @dknopik? 🙏 |
Issue Addressed
Mergify, in it's current config, adds PRs to the merge queue even if it is tested and up to date with the current branch. That is unfortunate because it adds additional waiting time for us.
Proposed Changes
Only add a PR to the merge queue if it is behind the target branch.
Otherwise, wait until tests have passed, and then merge directly.
Additional Info
CC @diegomrsantos