On Thu, Mar 24, 2011 at 9:27 PM, Happy-melon <happy-melon(a)live.com> wrote:
I think Roan hits it on the nose. Most of the
problems Ashar and Neil raise
are flaws in our code review process, not flaws in the tools we use *to do*
code review. I actually think that CodeReview works quite well, **for the
system we currently use**.
I think it works very poorly, independent of our code review process:
* The discussion system is buggy and unpredictably mangles markup (it
probably shouldn't have attempted to support wikitext in the first
place . . .)
* It doesn't allow line-by-line review of patches
* It doesn't allow sensible configuration of e-mail notification
* It doesn't integrate with version control beyond just reading it
(e.g., it could support merging to a branch from within the web UI)
* It doesn't integrate with bug tracking at all
These are only the things I can think of off the top of my head
without having even *used* decent code review software. I'm pretty
sure that if I had used something like Gerrit a lot, I'd recognize a
lot more drawbacks. The bottom line is, trying to write our own code
review software is just a bad idea in the long run.
Ultimately, though, it's a mistake to think of any
of these issues as
technical questions: they are **social** problems. We have to choose the
*mindset* which works for us as individuals, as a group and as a charitable
Foundation.
There are technical problems here as well. The technical advantages
of moving to a DVCS can be separated completely from the social
advantages of the new code review paradigms we could adopt after doing
so. Moving from SVN to git would be a step forward in a lot of ways
even if we kept the code review and deployment process basically
unchanged. But it's also a prerequisite for adopting certain types of
code review, or at least it would make adopting those types of code
review much easier, so we should really talk about switching to git
before we consider a review-then-commit system.
We know the regime which is at the
other end of the scale: the Linux kernel's universal pre-commit review,
which I'm going to suggest we call the 'Burnt Offering' approach to coding
as patches are worked, reworked, and inevitably reduced in number before
being presented for divine approval. That has clear advantages, in ensuring
very high code quality and probably improving *everyone's* coding skills,
but also the disadvantages Roan mentions.
The real distinguishing feature of Linux development isn't pre-commit
review, it's that it's pull-only and thus completely
individual-oriented. Linus Torvalds personally decides what gets into
the official Linux kernel, and no one else has any actual say (beyond
trying to persuade him). He mostly delegates to maintainers, who in
turn mostly run their parts of the kernel as fiefdoms as well. This
approach is idiosyncratic, and closely related to the fact that Linux
is produced by dozens of independent organizations with no central
organizational oversight.
I don't think we should be seriously contemplating the Linux model for
MediaWiki. The overwhelming majority of MediaWiki development is done
by either Wikimedia employees, or volunteers who are closely connected
to Wikimedia employees. MediaWiki isn't an individual's project, it's
Wikimedia's project, so a totally decentralized version control
process wouldn't match the reality of how development works.
I continue to suggest that we look at a process more like Mozilla's.
Like MediaWiki, Mozilla's projects are developed under the central
control of a not-for-profit organization (modulo wholly-owned
for-profit subsidiaries that exist for tax reasons) committed to
openness and community participation. It's much more accessible to
new contributors than either MediaWiki or Linux development, and I can
speak to that personally as someone who's submitted code to both
MediaWiki and Mozilla. (Not to Linux, but the reputation of Linux
development is consistently scary enough that I don't think I need
personal experience . . .)
The smoketest-trunk-every-week development model,
which defies being given a
crass analogy, is somewhere in the middle, and I think that's closer to
where we need to be. If we made an absolute policy of scapping to the WMF
cluster once a week, every week, it would force a shift in our mindset
(arguably a shift *back*), but not one that's seen as an artificial
limitation. No one will begrudge a release manager reverting changes on
Tuesday afternoon which people agree will not be fixed in time for a
Wednesday scap, while the same release manager spending Tuesday *not*
merging changes for the exact same reason is seen in a much more negative
light. We retain people's ability to make rapid and immediate changes to a
bleeding-edge trunk, but still ensure that we do not get carried away, as we
did for 1.17 and are still merrily doing for 1.18, on a tide of editing
which is not particularly focussed or managed (witness the fact that out of
the 15,000 revisions in 1.17, we can point out only about three 'headline'
features).
The key problem with this approach is that you retain the notion of
people who have "commit access" -- who are in the privileged position
of being able to submit their code without prior review. This creates
an artificial barrier to new users' contributions. They have to use
an entirely separate process to submit code, and it's very possible
that even if we get review of in-trunk code running smoothly, patches
on Bugzilla will remain neglected. After all, established
contributors aren't submitting patches on Bugzilla for review, so the
complaints about Bugzilla review problems will be few and quiet. Only
a unified review system, where new contributors and core developers
submit their code through the same process, will maximize
accessibility to new contributors.
Mozilla uses Bugzilla for this, which is frankly a pain in the neck,
but it works for them. Gerrit looks much better, although I have yet
to see it really in action.
Of course, under this system we'd still have people with something
resembling commit access. Mozilla does, unlike Linux. But commit
access would only be for if you've verified something has gone through
the proper review procedure, or for emergencies. So it would be a
bureaucratic right that makes your life slightly easier, not something
that lets you get code accepted more easily.
My main point is that *any* technical discussion,
about SVN/Git,
about CodeReview or its alternatives, even about Bugzilla/Redmine, is
premature unless we have reached an adequate conclusion about the social
aspects of this combined issue. Because Git does not write code, nor does
CodeReview or Bugzilla.
No, but git is a much better tool than SVN overall. We should switch
to it even if we have no intent of changing our development model.
Since a lot of us do want to try out changes to our development model,
that only strengthens the case for switching, since git is much more
flexible and can be more easily adapted to different development
models than SVN.
All this should only be after we get back to regular code deployment,
though. That's certainly the most important thing. First get code
review working with regular deployment, then switch to git, then
switch to review-then-commit, is what I'd like to see. (Although my
recent involvement with MediaWiki development has been so minimal that
what I'd like to see probably doesn't matter much, to me or anyone
else. I do hope to become more active again, but probably not for a
couple of years at least . . .)