I'd like to suggest a slightly stronger review process. In the draft guidelines <https://docs.google.com/document/d/1syFyfqIsGOYDE_Hn3ZkRd8a6ylcc64Kud9YtrGHgU0E/> it currently says all PRs should get reviewed, regardless of author. The only real exception to that is something that is an emergency fix, like rolling back a bad PR to fix a build break. Even then, use a PR for test coverage and communication -- just merge immediately.
Given that we're currently merging multiple disparate code bases, I think we'll benefit by increasing the number of folks familiar with the code. And in my experience, good quality code reviews are the best way to do that. And even when I own a chunk of code and think I know it inside out, code reviews still turn up subtle issues I missed. It's not a matter of trust, it's about building a strong community and spreading our knowledge to each other. I, for one, would love to learn more about Flink and Beam ;-) Frances On Mon, Mar 21, 2016 at 3:06 AM, Jean-Baptiste Onofré <[email protected]> wrote: > Hi Max, > > fully agree, my previous e-mail was more my own mea culpa as I pushed a > change without PR (of course it was minor change as it concerned the legal > NOTICE and DISCLAINER files ;)). > > So, my intend was just a global reminder, including myself ;) > > Regards > JB > > > On 03/21/2016 10:50 AM, Maximilian Michels wrote: > >> Hi JB, >> >> If I remember correctly from the past discussions, we agreed that we >> want a PR-review process for all code changes which are >> important/major or break the API in some way. I wholeheartedly agree >> with this process. >> >> In addition, committers preserve the right to provide small fixes >> which do not change important logic of the system. Ideally, this >> should only be changes that are part of the component which the >> committer is responsible for. In this way, we can focus on important >> pull requests and still provide meaningful fixes. It is the >> committer's responsibility to make sure those fixes don't hinder other >> committers' work. If necessary, committers should sync with each other >> before pushing fixes. In rare cases, commits can be reverted. >> >> Pull requests are a very important tool in the open source development >> process. Whenever possible, let's use them but please let's not >> enforce them. Community is also about trusting each other. Direct >> pushes should be used with care and shouldn't be the normal >> development process. On the other side, pull requests which get merged >> away immediately are not meaningful either. >> >> So far I think we are doing a good job. Please correct me if you feel >> differently. >> >> Best, >> Max >> >> >> On Sun, Mar 20, 2016 at 8:27 PM, Jean-Baptiste Onofré <[email protected]> >> wrote: >> >>> Hi all, >>> >>> As a reminder, we agreed that everyone in the project should use the same >>> workflow: prepare a PR, submit the PR, give some time to review, apply >>> the >>> PR. >>> >>> Right now, we had some pushes directly without a PR. >>> >>> It would be great to we *all* use the same workflow. >>> >>> Thanks ! >>> >>> Regards >>> JB >>> -- >>> Jean-Baptiste Onofré >>> [email protected] >>> http://blog.nanthrax.net >>> Talend - http://www.talend.com >>> >> > -- > Jean-Baptiste Onofré > [email protected] > http://blog.nanthrax.net > Talend - http://www.talend.com >
