Publishing code reviews
Phil.Race at Sun.COM
Thu Oct 11 18:09:08 UTC 2007
Dmitri beat me to it ..
'robot' also archives the webrevs and the email exchange .. so
even the rest of the world who really didn't want to see every
set of comments on every bug can always go look ..
It also generates approval emails with nicely formatted putback comments.
I was chatting yesterday with an external developer who saw
something like this as the next step after mercurial.
BTW Mark wrote :
> In mainline
> JDK development at least one review is required for every code change,
> and two or more are needed as release milestones approach.
In the client groups (2D, AWT, Swing etc) we always require two reviews,
except for the most trivial and low-risk changes (updating a test and the like,
which optionally may only require one review). I expect we will continue
to maintain that standard.
Dmitri Trembovetski wrote:
> Hi Mark,
> what about moving one of our our internal code review tools
> to an outside server?
> Like the one the client team uses - the 'code review robot'.
> You submit the code review either via email or web page.
> The webrev is then hosted on the "robot system" so the reviewers
> can see it.
> The communication between the reviewers happens via email, the
> robot is CC-ed. The tool supports multiple fix revisions,
> approvals, etc.
> It will probably need some polishing up - since currently
> there's no authentication, for example, but it's been used
> by many people for a long time and is found to be very
> convenient if not as flashy as Mondrian and some ajaxy tools..
> Mark Reinhold wrote:
>> 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: ). webrev was originally written
>> by the Solaris team; lately it's been revised and extended  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 ,
>> 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?
>> - Mark
>>  http://cr.opensolaris.org/~dp/i2o-del/
>>  http://blogs.sun.com/dp/entry/webrev_revised
>>  http://cr.opensolaris.org/
>>  http://blogs.sun.com/dp/entry/cr_opensolaris_org_a_work
More information about the discuss