-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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.
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
Automatically merge green backport PRs and green approved PRs #41110
Conversation
executing it locally. Would somebody approve it so I can test the script? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing approve
Oh dear, the logic of taking the latest review per user isn't a good idea |
Hmm, doesn't look like |
I think it's OK to have approval from collaborators as a marker that the PR is generally ok. update: we discussed internally, and the script will consider only members' reviews |
Oh no, the PR would be merged on Finish in any case, even if there are failed statuses. It needs another check |
4de6809
to
55e3d61
Compare
Maybe it also worth to check:
Actually right now github provide everything similar:
The only problem is that you have to have write access to merge PR, so this is great, but will not work. Actually I found auto-merge. So maybe all of this can be simply replaced with configuring proper policies on github? And AFAIU anybody can select it, and this looks better then simply auto merge, maybe an author will find more problems with the patch. P.S. llvm uses different policy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have calculated self.approved_at
without distinguishing the author_association
. So, someone outside of the organization can approve after commits have been made after another legitimate approval.
26d2bdd
to
9398b7e
Compare
We can limit the recognized approvals to the core team. |
@azat the GH's approach is pretty dumb (not in a bad way, just simple) and straightforward.
These two don't allow us to consider the solution, it just won't work for us |
168ffea
to
5a84f9c
Compare
Backport #41110 to 22.12: Automatically merge green backport PRs and green approved PRs
Backport #41110 to 22.10: Automatically merge green backport PRs and green approved PRs
Backport #41110 to 22.8: Automatically merge green backport PRs and green approved PRs
Backport #41110 to 22.3: Automatically merge green backport PRs and green approved PRs
Backport #41110 to 22.11: Automatically merge green backport PRs and green approved PRs
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Automatically merge green backport PRs and green approved PRs, fixes #40325