On Sun, Feb 13, 2011 at 8:11 PM, Mark A. Hershberger
<mhershberger(a)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.