Tim Starling tstarling@wikimedia.org writes:
On 26/03/11 14:56, Mark A. Hershberger wrote:
If code is to survive past a week in the repository, it has to be reviewed.
If you want to make a commit that depends on un-reviewed code, you have to find someone to review it. Otherwise, your commit will break trunk when that code is reverted.
Find someone to review it? If the experienced developers on the WMF payroll aren't assigned to code review, then under your proposal, the only option for avoiding a revert will be to get someone with no clue about anything to rubber-stamp the code.
Thanks for pointing out the things I hadn't considered in my suggestions. I was focused on making junior developers motivated to find reviewers, but neglected to thoroughly consider the results of my suggestion.
Your proposal seems to be based on the idea that review under Git is many times better than review with CodeReview and Subversion. I don't think that's true, I think it's very slightly better. Whether you use Git or Subversion, you still need people with brains reading code.
To be clear, I don't think Git is vastly superior to any other VCS for getting code review done. I do think that since Gerrit, for example, appears to be more widely used and supported than than MediaWiki's CodeReview extension, that many people come to this conclusion.
I could be wrong. I'm probably am. But I'd like to fix the Code Review problem so that it can no longer be used as an excuse to change our VCS.
Using Subversion has its pluses and minuses. Code review should not be one of them.
Mark.