Intial proposal #1
Conversation
|
Thanks for the PR @isnotinvain! |
|
Thanks @sriramkrishnan for starting the doc + discussion this is taken from! |
|
|
||
| ## Code Review and Merge Process | ||
|
|
||
| Committers (and other contributors) will review your contribution (i.e. a pull request). Each contribution must get at least one +1 from an active committer (who is not yourself) before merging. If the initial reviewer is unsure whether to +1 something or not, they should involve other committers for testing/validation of the contribution. No PRs should be merged if there are open issues that have not been discussed, or if there are any -1s without further discussion, or if the build is not green. Any committer can merge the change after the review process is complete and the build is green. |
There was a problem hiding this comment.
do we want to explicitly mention how long we should wait after a +1 (so that other committers can take a look?)
There was a problem hiding this comment.
Maybe it's up to the first committer that reviews? If all they say is +1, then I think it's ready to go. But if they say +1 for me but maybe <person> should take a look, then you should wait some amount of time. We could say 3 business days.
There was a problem hiding this comment.
Yeah I guess I was getting at the scenario where the committer might not explicitly mention one of the other committers to review. Might be a good idea to mandate waiting a day or two (unless its an urgent fix) to ensure in case anyone else has concerns they can voice them before it gets merged.
There was a problem hiding this comment.
Anyone else want to weigh in? We could do 1 business day? I don't think we've done this in the past though, so I don't know if we need to introduce a new rule?
There was a problem hiding this comment.
reverts are not expensive. If we don't have a problem, maybe bias to speed?
There was a problem hiding this comment.
Yeah, I agree. We can always revert if there's further discussion needed. And the committer who +1s should mention if further discussion is needed before +1-ing
|
Do we have more to discuss, or are we ready to merge this? |
|
👍 |
Here's an initial proposal for the governance doc. Lets address any remaining issues in this PR.
After we are happy with this, I propose that we create a COMMITTERS.md file in each project repo, seeded with Oscar, Ian, Alex, Ruban, Sriram, Piyush, Pankaj, Joe, Ben (Ian, Oscar + Twitter CDL team). Additional committers can be added on a per project basis after that.
Let me know what you think, thanks!