docs icon indicating copy to clipboard operation
docs copied to clipboard

"Require review from Code Owners" branch protection clarification

Open aergonaut opened this issue 3 years ago • 3 comments

Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-pull-request-reviews-before-merging

What part(s) of the article would you like to see updated?

The final paragraph in the section "Require pull request reviews before merging", which talks about the "Require review from Code Owners" branch protection setting, is unclear about what happens if a file has multiple code owners.

The meaning of the setting is somewhat ambiguous. It could be read to mean that ALL code owners for a file are required to approve a change. Since all of the code owners are requested for review when a PR modifies a file with multiple owners, and since they are all marked with the code owner icon, it could be assumed that the branch protection setting would also require all of them to approve.

The reality is that the setting only requires at least one code owner for every file to approve.

It would be good if the docs state this directly.

Additional information

We have been investigating GitHub's code review features for some time now. This was a question that arose to which we couldn't find a direct answer in the documentation. We ended up trying it out on a test repository, but I think it would be helpful for the docs to state the behavior directly as well.

aergonaut avatar Apr 06 '22 22:04 aergonaut

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

welcome[bot] avatar Apr 06 '22 22:04 welcome[bot]

@aergonaut Thanks so much for opening an issue! I'll triage this for the team to review :eyes:

ramyaparimi avatar Apr 07 '22 13:04 ramyaparimi

@aergonaut - thanks for raising this ambiguity 💖

It's difficult to be explicit in the docs because there are so many possible branch protection settings and some of them interact, but I think that your description is spot on. I'll confirm this with an internal team, and then we can decide what docs changes are needed to make this clear and create a content design plan for those changes.

felicitymay avatar Apr 11 '22 09:04 felicitymay