Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

merge on green: 'automerge: exact' catches up branch and allows MoG merge commits #5096

Closed
noahdietz opened this issue Jul 5, 2023 · 0 comments · Fixed by #5333
Closed

merge on green: 'automerge: exact' catches up branch and allows MoG merge commits #5096

noahdietz opened this issue Jul 5, 2023 · 0 comments · Fixed by #5333
Assignees
@chingor13
Labels
bot: merge on green priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@noahdietz
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I want an external contribution that I've approved to be merged automatically with automerge: exact (so that the contributor does not sneak an extra commit in post-approval/pre-automerge), but without having to reapprove whenever the branch is caught up by an automatic merge-commit (e.g. from MoG merging main into the branch). In a high traffic (lots of merges) repository with long running presubmits (kokoro, on the order of tens of minutes), each time I have to catch up a PR branch, it requires I reapprove the PR to satisfy automerge: exact. This is toil.

Describe the solution you'd like
I would like MoG to do the following with automerge: exact:

  1. Automatically catch up the branch (as automerge already does)
  2. Not-require reapproval from reviewers if the latest N commits since last approval are merge commits executed by MoG itself.
  3. Automatically merge the PR

Describe alternatives you've considered
Use automerge - this might be fine since I think external contributions require that Kokoro presubmits be manually run by maintainers, but that ties us to Kokoro, something we may want to get away from in the future, and that not all repos use.

Babysit the PR with automerge: exact, manually catching up the branch and reapproving. This is just toilsome but not the worst.

Additional context
The PR in question was googleapis/google-cloud-go#8196, in google-cloud-go, a mono-repo that has decent commit traffic and slow(er) presubmits.
@noahdietz noahdietz added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. bot: merge on green priority: p3 Desirable enhancement or fix. May not be included in next release. labels Jul 5, 2023
@sofisl sofisl mentioned this issue Jun 6, 2024
Merged
sofisl added a commit that referenced this issue Jun 11, 2024
@sofisl
fix: [auto-approve] add event listener for pull request dismissed (#5333
f4d9b27 
)

I noticed in googleapis/nodejs-pubsub#1927 that
merge-on-green kept dismissing auto-approve's latest review due to
post-processor updates, other commits, etc. Ideally, those commits would
retrigger auto-approve to re-review the PR. But there seems to be a race
condition where sometimes merge-on-green beats auto-approve to
re-approving and dismisses the PR. This fix would allow auto-approve to
listen to reviews dismissed, so that it always has a chance to re-review
the most recent commit when an `automerge: exact` label has been placed.

As part of this work, the pull_request_review.dismissed event needs to
access the pr number through
[`pull_request.number`](https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_request_review),
so I had to update this for all the events and make sure the test
fixtures had the appropriate number there too.

Fixes #5096
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chingor13 chingor13

Labels
bot: merge on green priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: [auto-approve] add event listener for pull request dismissed googleapis/repo-automation-bots
2 participants
@chingor13 @noahdietz