Publishing code reviews

[Andrew Haley]

Mark Reinhold writes:
 > One of the engineering practices that's served the JDK development team
 > well for many years now is that of peer-based code reviews.  In mainline
 > JDK development at least one review is required for every code change,
 > and two or more are needed as release milestones approach.
 > 
 > Reviews are most often performed with the help of a tool called "webrev",
 > which basically diffs two source trees and generates a pile of HTML that
 > you can view in a browser (sample: [1]).  webrev was originally written
 > by the Solaris team; lately it's been revised and extended [2] to support
 > Mercurial and Subversion in addition to the old internal TeamWare SCM.
 > 
 > A lot of the e-mail that JDK developers read and write is, naturally,
 > about code reviews.  It's not exactly convenient to attach webrevs to
 > e-mail messages, so people typically either copy them to a directory
 > published by a private web server and send the URL around, or they copy
 > them over to an NFS-exported filesystem and send a long pathname (and
 > cross their fingers if a potential reviewer is far away network-wise).
 > 
 > The bug here, of course, is that these publication methods don't make
 > code reviews visible on the open web, and this is one reason that most
 > JDK developers have thus far been reluctant to move more of their e-mail
 > traffic onto the public openjdk.java.net mailing lists.
 > 
 > So: How should we solve this problem in a way that works for all JDK
 > contributors, whether or not they work for Sun?
 > 
 > We could build something slick and fancy, like Google's Mondrian [3],
 > but I think it's relatively more important to get something up quickly
 > and work on improving it later.
 > 
 > A more expedient solution would be to construct something like the
 > OpenSolaris code-review site [4,5], where contributors can use rsync,
 > scp, and sftp (all via SSH) to upload webrevs to temporary storage on
 > a public server.  SSH keys will ultimately be required in order to push
 > changesets into the public Mercurial repositories, so contributors will
 > need to create and register SSH keys anyway, and the code-review site
 > can easily leverage the same underlying infrastructure.
 > 
 > Comments?  Alternatives?

Here's a really simple suggestion: convert the diffs to "diff -u"
format and email them to a list.  The list gets archived, so all the
diffs are available forever.  There doesn't have to be one single
format for a diff: some people may continue to use webrev, but others
may look at simple diffs.  Also, search tools such as Google work well
on archived mailing lists.

It's important to reduce the barriers to entry for people wanting to
work on OpenJDK, and while full-time contributors may well be highly
motivated to learn new tools, more casual browsers will be deterred by
having to do so.

>From looking at the sample in [1], while it is attractively presented,
it's not clear to me that the nice formatting is sufficient to justify
the disadvantage to a user of not being able to use the mail reader of
their choice.

Another issue is long-term archiving: I have quite often gone back
five or ten years to look at a particular gcc patch to see what
problem it was supposed to solve and read the patch reviews.  This
doesn't depend on any one server because gcc discussions are archived
on many servers run by different organizations.  Being able to reach
back in time is a vital part of the open development process, and you
don't want to depend on any one organization's server infrastructure.

Finally, a server that only keeps webrevs for a limited period of time
is a bad idea.  We need to be able to find every version of every
patch that has ever been proposed.

Andrew.

-- 
Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, UK
Registered in England and Wales No. 3798903