GitHub Community
Categorized PR review policies #22561
-
|
I am quite sure I am not the first to run into this issue, so we’ll see how others react. At the moment (and please correct me if I am wrong), you can only tell github that PR has to be reviewed before merging, or not. This black and white policy doesn’t really work in the real world for me and my colleagues. Specifically, PR’s are not all painted the same color. Some are trivial, some are a little more complex, some touch things that should not be touched without a good plan and review. Allowing things to go through without review is dangerous for most orgs, obviously, but it would be nice to be able to define a scheme for review policy, such as classification of PR’s similar to incidents, P1, P2, P3, etc. P3’s are trivial changes that may require no review because they are, for example, documentation changes only. P2’s are more complex reviews that only require 1 review while they change something in the target, it’s not really something that can blow up in your face. P3 or higher are for changes that touch things that could make life difficult for people, like changing the docker defaults file and causing all docker agents to be restarted. Not good. But right now, I don’t know we can do this kind of categorization. Does this make sense to anyone else? I’m open to comments, flames, modifications of the strategy, whatever. :slight_smile: |
Beta Was this translation helpful? Give feedback.
Replies: 4 comments
-
|
" P3 or higher are for changes…" Sorry, I meant P1’s are for changes…, |
Beta Was this translation helpful? Give feedback.
-
|
You are right that this is the current policy and that this can’t really be changed. You could never require a review , and then use the P1, P2 and P3 as issue labels so people know when merging is allowed. There currently is no other way to accomplish it otherwise with just GitHub. A couple of things I thought of to still accomplish your goal:
|
Beta Was this translation helpful? Give feedback.
-
|
This is not a solution because it makes the reviews optional. That is completely the opposite of what is desired. |
Beta Was this translation helpful? Give feedback.
-
|
On this forum posts that seem to be a solution are automatically marked as a solution if the thread becomes inactive, which was the case for this thread. I’m sorry you did not find a suitable solution. Can you explain why a CODEOWNERS file would not work for you? |
Beta Was this translation helpful? Give feedback.
You are right that this is the current policy and that this can’t really be changed.
You could never require a review , and then use the P1, P2 and P3 as issue labels so people know when merging is allowed. There currently is no other way to accomplish it otherwise with just GitHub.
A couple of things I thought of to still accomplish your goal: