On Sun, Feb 13, 2011 at 8:11 PM, Mark A. Hershberger mhershberger@wikimedia.org wrote:
This workflow is different from a DVCS. Take Linux, for example. Linus pulls code from several lieutenants. Anyone can set up a branch of the Linux source code and commit to it, but to get Linus to ship their code, they have to get a lieutenant to review it and give it to Linus.
The workflow is different in that code review is an integral, not peripheral, process. As a bonus development access is available to anyone willing to run “git clone”.
On the other hand, in Linux, it can be hard to get patches reviewed and accepted in a timely fashion, because there's no clear chain of command unless Linus personally intervenes. I think Mozilla is an excellent model to follow, in that they have a well-defined review process (not "everyone can object to design decisions at the eleventh hour" like Linux) and make sure that all patches get reviewed in a timely fashion (at least as far as I've seen). I've submitted two patches to Mozilla, and I got immediate feedback and review on both of them from developers responsible for the relevant areas.
What I think is important is that a) there's a formal process that ensures all submitted code gets reviewed, and b) this process is basically the same for everyone (with no group of people with "commit access" who get to jump the queue). Without (b), code by new contributors will too easily slip through the cracks. Mozilla has everyone submit code on Bugzilla, which is awkward but works -- even core developers have to file a bug, submit a patch there, and get review from another qualified coder, essentially the same as anyone. (Obviously with exceptions like backing out build breakage and so on.) Mozilla does have people with commit access, but they're just the ones tasked with the chore of checking in code once it's been reviewed -- they aren't allowed to just check stuff in without going through the review process first.
If we switched to git, perhaps we should take a look at Gerrit for a review tool. I've heard good things about it, but never used it myself. Whatever we use, I think it should really be "review then commit", not "commit then review". Cherry-picking from trunk to a branch is possibly better than reverting things in trunk (not sure), but in the medium term it would be much better to get rid of commit access status entirely -- once we're sure we have a properly-functioning review process that can keep up with changes.