On 20 January 2014 12:26, Dale Harvey <[email protected]> wrote: > I fully agree, its something I mentioned at the couchdb conf in vancouver, > a review first system encourages contributions and has multiple benefits > > * At least 2 people look at the code, less likely to push silly mistakes > * Can codify and practice review rules > * Its much easier to view the current activity in the project > * Can bring up points of discussion before its too late > > But I think the most important thing is that it removes the burden of > responsibility from the committer to the project as a whole, also hugely > beneficial is that it makes it particularly obvious when a patch has reach > a stalemate and forces someone to make the call. > > For reference on PouchDB every committer is trusted to push code, nobody > (including myself) pushes their own code to master, it goes in the PR queue > and gets a +1 from any other committer (who will usually push it), thats > essentially the same process we use at mozilla and coming to think of it > any other project I have worked on, any commiter has the ability to -1 a > patch at which point they give a reason and usually some solution gets > agreed on >
I like this system, with one small quibble about responsibility. I don't think it should be seen as removing the burden of responsibility from the committer to the project as a whole. It becomes easy then for everyone, including the committer, to think someone else will find bugs, and no-one puts in enough effort. I'd say it is still primarily the responsibility of the committer to ensure that code is error free, but that at least one person who knows that area of the code base should sign off on it. Some spreading of responsibility is good, but too much can actually lead to a decline in quality. Nick
