GitHub Community
[Feature request] Allow PR author to "require" all reviewers to approve #48162
Replies: 27 comments 4 replies
-
|
I would love something like this, though I'd much prefer the ability to specify specific reviewers as required, rather than making it all or nothing. Happy to settle for all or nothing. With large teams there are often a handful of reviewers who get added and satisfy a single "codeowner" requirement, if we get the "all or nothing" model I think it's important that a single one of the auto-added reviewers still satisfies the requirement. My primary use-case is when I'm reviewing a PR but I want to require that design take a look before the PR auto-merges. I can mention them in my review, but there isn't any way to encode that into the PR itself right now. A hack I've considered is adding a file which, when touched, requires a specific team's review on the PR, like:
#1234
+ #1235 |
Beta Was this translation helpful? Give feedback.
-
Agreed, this gives more flexibility! |
Beta Was this translation helpful? Give feedback.
-
|
I've added a mockup:
|
Beta Was this translation helpful? Give feedback.
-
|
Wow @dvdsgl that looks really gorgeous. Maybe the most beautiful checkbox design I've ever seen. 🎨 This is the only other competitor design I've seen so far:
|
Beta Was this translation helpful? Give feedback.
-
|
@shayfrendt fixed that for you:
|
Beta Was this translation helpful? Give feedback.
-
|
@dvdsgl , this is cool but would still require admin control of the repo. The optimal would be to be able to request this as a PR author! But yeah, that's a start: with that setting, at least one would know that IF you request approval from someone, then they must approve. Other people can be CCd instead |
Beta Was this translation helpful? Give feedback.
-
|
we're also looking for something like this. all-or-nothing would be fine for us. repo-admin deciding would also be fine for us. @github team: this is a feature of BitBucket. if you implement it quickly enough, you might grab some customers. BitBucket server is coming to end of life in Feb 2024. |
Beta Was this translation helpful? Give feedback.
-
|
link collection for other discussions about this topic - please feel free to reply and I will update this posting :)
|
Beta Was this translation helpful? Give feedback.
-
|
Any chance this is being worked on? |
Beta Was this translation helpful? Give feedback.
-
|
Cross posting my duplicate feature request here to consolidate the interest and discussion in one place. I did not see this post when I made mine. Same end goals. Same suggested user experience (although I would add one more row of checkboxes to the proposed UI .. :) Update since I made my original post: we now use a custom action to provide this functionality. It works, but has some rough edges. For example all pull requests look like they have failed until all reviewers have approved. That ends up creating a "sky is falling" scenario and now no one takes a CI failure on a PR seriously, which ruins our engineering system in a different way. The custom action has convinced me this is an important feature. But it has also convinced me that the only way to do it correctly is if it is a built-in github feature. Select Topic AreaProduct Feedback BodyProblem StatementWhen working with github in a team setting, a lot of pull requests have a single reviewer, and as soon as that reviewer approves we merge the pull request. That makes sense. But when multiple reviewers have been selected, we sometimes find that pull requests are merged after a single reviewer has approved but another reviewer is in the middle of reviewing. This is mostly a coordination problem, but sometimes tooling is the best way to coordinate. Branch protection rules today allow pull requests to have a sepcified number of approvals before being merged. I currently use that feature, and have that set to "1" to ensure a review has happened. I could use this feature to ensure a second (or third, or fourth) reviewer gets a chance to complete their review, but then every pull request would require two/three/four reviews, and we have a lot of pull requests that only really need one review. Feature RequestWould it be possible to have another branch protection setting that is similar to this: block merging until all reviewers have approved? BenefitWith this new requested feature, if a pull request lists one reviewer, then only that one approval is needed in order to merge. If two reviewers were identified, both would have to approve before the pull request could be merged. This allows simple pull requests to stay simple, but ensures that more complicated pull requests get reviews from all the people who have expertise about the change (or who are impacted by the change). CommentaryI could see this new feature and the existing "block merging until N approvals have been given" feature interacting in a complementary way, for example:
ReferencesThis feature request is similar to, but not exactly the same, as what is discussed in https://github.com/orgs/community/discussions/5289 |
Beta Was this translation helpful? Give feedback.
-
|
I would also find this useful. I would like to be able to specify a list of people or teams in branch protections or rulesets that must approve before a PR can be merged. If a team is selected, it would be nice if I could specify the minimum number of people from that team that must approve. |
Beta Was this translation helpful? Give feedback.
-
|
+1, this would be a solid add. |
Beta Was this translation helpful? Give feedback.
-
|
We could use a "Require all reviewers to approve" checkbox, too. |
Beta Was this translation helpful? Give feedback.
-
|
+1, would be very happy to have this. |
Beta Was this translation helpful? Give feedback.
-
|
Either this feature or specifying mandatory vs optional reviewers would help us. Gitlab, Bitbucket and even Azure DevOps allow you to specify optional vs mandatory, so I just assumed Github had it too. When you assume... |
Beta Was this translation helpful? Give feedback.
-
|
+1, this is needed |
Beta Was this translation helpful? Give feedback.
-
|
+1 please add :) |
Beta Was this translation helpful? Give feedback.
-
|
+1 please add! |
Beta Was this translation helpful? Give feedback.
-
|
+1, this feature should be added! If it's sensitive code that needs to be approved by all reviewers, it should be done. Such code should not be allowed to merge after one approval out of six (of three, of four, ..., for example). |
Beta Was this translation helpful? Give feedback.
-
|
+1, please add this feature. We would find it SUPER useful. :) |
Beta Was this translation helpful? Give feedback.
-
|
+1, please add this feature |
Beta Was this translation helpful? Give feedback.
-
|
+1, please add! |
Beta Was this translation helpful? Give feedback.
-
|
+1, please add this feature |
Beta Was this translation helpful? Give feedback.
-
|
+1, Feature would be really helpful. Please add. |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
|
We have this in ADO. This is a critical feature! |
Beta Was this translation helpful? Give feedback.
-
|
Que Pedo con lo que se espera y quedamos ?? |
Beta Was this translation helpful? Give feedback.
and others
-
Select Topic Area
Product Feedback
Body
AFAIK, there is a setting (if you're an owner) for a minimum number of approvers, but that's not what I'm talking about. But, sometimes, PR authors (i.e. me) want to receive approval from all reviewers before merging.
However, right now:
Would it be something useful to explicitly mark the PR as "wants-all-reviewers" to approve or something like that?
Beta Was this translation helpful? Give feedback.
All reactions