On 29/03/11 11:26, MZMcBride wrote:
> Long ago I lost track of who's in charge of what, but I'm told there is some
> sort of hierarchy in place in the tech department. Who's empowered to start
> assigning people to review code in a reasonable timeframe? Like Aryeh, I
> find this entire thread a bit baffling.

The hierarchy is CTO -> EPMs -> regular plebs like me. The EPMs are
Rob Lanphier, CT Woo, Mark Bergsma and Alolita Sharma. General
MediaWiki work is mostly Rob Lanphier's responsibility, which is why
he's been so active in this thread.

Rob doesn't know as much about MediaWiki as me, but he knows the
people who work on it and how to manage them. I think his response
with subject "The priority of code review" was entirely reasonable.

I'm not saying that I think MediaWiki code review should be the
highest priority task for the Foundation, or that it's important to
review all commits within a few days, as Aryeh contends:

Aryeh wrote:
> If commits are not, as a general rule, consistently reviewed within two
> or three days, the system is broken.  I don't know why this isn't
> clear to everyone yet.

I'm saying that the Git/Subversion debate is peripheral, and that
human factors like assignment of labour and level of motivation are
almost entirely responsible for the rate of code review.

In the last week, I've been reviewing extensions that were written
years ago, and were never properly looked at. I don't think it's
appropriate to measure success in code review solely by the number of
"new" revisions after the last branch point.

Code review of self-contained projects becomes easier the longer you
leave it. This is because you can avoid reading code which was
superseded, and because it becomes possible to read whole files
instead of diffs. So maintaining some amount of review backlog means
that you can make more efficient use of reviewer time.

Our current system links version control with review. After a
developer has done a substantial amount of work, they commit it. That
doesn't necessarily mean they want their code looked at at that point,
they may just want to make a backup.

It's useful to look at such intermediate code for the purposes of
mentoring, but that's not the same sort of task as a review prior to a
tarball release or deployment, and it shouldn't have the same priority.

-- Tim Starling


_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to