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.

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

Update PR approval requirements #1623

Open
SWilson4 opened this issue Dec 6, 2023 · 1 comment
Open

Update PR approval requirements #1623

SWilson4 opened this issue Dec 6, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@SWilson4
Copy link
Member

SWilson4 commented Dec 6, 2023

Preserving @baentsch's suggestion from #1618 so that it doesn't get buried after the merge:

I don't know how to change the "2 approvals needed" rule to only apply to certain directories -- we can apply it only to certain branches, but I don't see directories as an option.

Hmm -- just read the documentation and I also didn't find a way how to do that. A bit disappointing, I'd say. But what I did find is an option that demands approval by code owners. And that can be set to specific directories, right? So what about the idea to carefully define code ownership(s), set this flag and revert back to just 1 review needed for PRs? Not ideal if there's a sole code owner designated for some sections and that person isn't around -- in such case the 4 eyes rule would be better again...

@SWilson4 SWilson4 added the enhancement New feature or request label Dec 6, 2023
@planetf1
Copy link
Contributor

Adding users in CODEOWNERS is good practice regardless, as it should help to automatically assign reviewers. It's worth ensuring >1 owner on each area.

Having 2 reviewers is nice, and arguably important in security code as even if one is the owner, something could be missed. the question is whether it slows development down too much, especially in a small repo

Quite apart from the above, any exceptions to current or future policy could be implemented by having a github action that 'auto-approves' based on a specific encoded policy. So the basic rule would be tight, and then this allows some consessions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

No branches or pull requests

2 participants