Re: [Wikitech-l] The priority of code review (Re: Code Review tools)

On Mon, Mar 28, 2011 at 4:31 PM, Rob Lanphier <robla(a)wikimedia.org> wrote: If you let months and months of code review pile up, then yes, people will have to do code review full-time to catch up. But if you review all code as it comes in, and you have a sensibly large pool of reviewers, no one person will have to spend more than a moderate amount of their time (much less than half) reviewing code. You don't have to limit review to only really senior people -- most developers should be competent enough to review patches to code they wrote, for instance. Reviewing code normally takes *much* less time than writing it. If reviewing code takes a fifth the time that writing it does, then any volunteer-submitted code is available for Wikimedia use at one-fifth the regular cost. Even if it's something Wikimedia wouldn't normally have devoted resources to develop, it's likely to at least be worth the resources to review. Code that's entirely useless to Wikimedia is usually going to be code that doesn't run on Wikimedia sites, which doesn't need to be reviewed at all. Plus you get the benefit of using volunteers as a sort unpaid interns, whom you can later decide to hire. Really, though, this is a matter of fact, not opinion. What reason do you have to believe that code review will actually take up so much time? Lots of projects accept volunteer contributions and ensure they get promptly reviewed, like Mozilla, WebKit, and Chromium. It works well for them; why not for us? It's not like Google is interested in paying expert developers to waste all their time reviewing useless contributions to Chromium, or likewise for any similar project. For instance, as one data point, how many developer-hours were needed to clear each week of commit backlog? It must be a lot less than forty times the number of developers assigned to it, or you wouldn't have been able to clear the backlog at all. If particular developers dislike code review, they can be given less code review to do and more coding. It's also not necessary to assign only the most senior developers to code review. The workload should be distributed in whatever way makes sense. But really, if the problem is that it's the senior developers who don't want to review code, then I agree, that's a problem. Is that the case? The post by Tim Starling that I referred to suggested that he felt experienced developers should be assigned to do more code review than they are, and that it was management that wanted to de-prioritize it rather than the reviewers themselves. Roan and Brion have also both stated pretty clearly that they want to see a return to prompt review and regular deployment. That's three senior developers right there. Which are the ones you're afraid will burn out? On Mon, Mar 28, 2011 at 6:28 PM, Trevor Parscal <tparscal(a)wikimedia.org> wrote: Of course very little will get done if you let a huge backlog accumulate. If you review code on a regular, sustainable basis, there will be plenty of time to do other things, especially if reviewers are judicious in rejecting patches that they think are too difficult to review for the benefit. Put another way, the fact that paid developers could clear out a year's backlog working more or less full-time for much less than a year demonstrates that to keep up with regular review would require much less than full-time work.