On Wed, Oct 14, 2009 at 6:09 PM, Brion Vibber brion@pobox.com wrote:
Throwing in a huge number of committers at once is problematic as well since that means more time is going to be spent in post-commit review; new committers generally need some time to really ramp up.
Pre-commit review -- eg checking then committing patches -- takes longer, but is safer. It's a balance that needs to be struck and yeah, it's tricky.
Ideally we'd have a much faster, cleaner pre-commit review pipeline and keep the number of general core committers relatively small. (We wouldn't have nearly as many committers as we do -- over 100! -- if patch review were consistent.)
Given the existence of wmf-deployment and release branches, though, the distinction between pre-commit and post-commit review seems mostly academic. MediaWiki trunk is pretty much just a place where people believed to be reasonably sane can throw up code for broader review and inspection prior to actual deployment or release. Having a central trunk with lots of committers reduces problems with merge conflicts and gets patches tested earlier in the development process, compared to a pre-commit-review system.
Post-commit review is a big problem for us, but it's because nobody likes reviewing code and not many people are really qualified to, anyway. Places like Mozilla and Linux that do pre-commit review have a much larger number of developers paid to do review, as far as I can tell. From what I've seen, with Mozilla, you assign your patch to someone and they actually look at it fairly quickly. Linux is its own crazy world, which we need not even talk about as long as we're not using a DVCS, but a lot of people there (like Linus Torvalds) spend much more time reviewing than coding, as I understand it.
As long as we have only a couple of people who are a) entrusted with reviewing code for deployment/release and b) paid to do so, I imagine review will remain a problem. Shutting out new committers is one way to mitigate it, but it's not a very healthy way for MediaWiki. At this point, I'd imagine one person paid to do full-time review would be enough to get us back to a reliable weekly deployment schedule with time left over, even if we add a bunch more committers.
Then again, I say all this as someone who hasn't even had time to look at the last 1000+ commits. :(