Publishing code reviews

[Joseph D. Darcy]

Phil Race wrote:
> 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.

Yes, we've also used the robot on the javac team.  It is easy to use and 
helps manage a large flow of review requests.  The archival aspect is 
important too.

-Joe

> 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.
>
> -phil.
>
>
> 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..
>>
>>   Thanks,
>>     Dmitri
>>
>>
>> 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: [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?
>>>
>>> - Mark
>>>
>>>
>>> [1] http://cr.opensolaris.org/~dp/i2o-del/
>>> [2] http://blogs.sun.com/dp/entry/webrev_revised
>>> [3] 
>>> http://www.niallkennedy.com/blog/archives/2006/11/google-mondrian.html
>>> [4] http://cr.opensolaris.org/
>>> [5] http://blogs.sun.com/dp/entry/cr_opensolaris_org_a_work