-
-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Add block on pending codeowner reviews branch protection #34995
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
base: main
Are you sure you want to change the base?
Conversation
7ff6bd4
to
7141ade
Compare
39ba543
to
18cce17
Compare
a5035a7
to
5df3166
Compare
|
||
hasApprovals := true | ||
|
||
for _, rule := range rules { |
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.
The logic appears overly complex and doesn’t seem correct. I ran a test, and it didn’t work as expected. Could you please rewrite it to make it clearer and more readable?
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.
I ran a test, and it didn’t work as expected.
Can you give me more info about that, like the codeowner file you used and what you expected vs. what happened?
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.
Also went and amended it to be a bit more concise. The behavior should be unchanged though, so I'd still be interested in your findings.
9b575f8
to
b448f2c
Compare
b448f2c
to
e01ac3d
Compare
This commit introduces a new branch protection rule that allows merge blocking if there are pending reviews from one or more code owners (as defined in any valid
CODEOWNERS
file). This is determined by evaluating each rule present in theCODEOWNERS
file individually. For every rule, at least one named code owner (or member of a code owner team) must have given an approving review for merging to be possible.Closes #32602
This PR does NOT display code owners separately from other reviewers (#28137), as I think it makes sense to do that in a separate PR, as it doesn't require any database migrations.
Screenshot
Pull Request
Branch Protection Rule
Unresolved questions