Draft Public Code Review Proposal

[Edvard Wendelin]

Hi,

On 3 aug 2011, at 09.05, David Holmes wrote:

> Hi Dalibor,
>
> Dalibor Topic said the following on 08/03/11 13:03:
>> On 7/14/11 7:00 AM, David Holmes wrote:
>>> What is the process for obtaining approval?
>> Send an e-mail to jdk7u-dev at openjdk, with
>> Subject: Request for approval for CR $NR
>> With the body containing a) a link to the publicly visible bug on  
>> the bugs.sun.com site (or its equivalent), or a description of the  
>> change in case no publicly visible bug is here
>> b) a link to the publicly visible webrev or changeset (in case it's  
>> in JDK 8 already)
>> c) if the review is taking place somewhere else, a link to the  
>> public review thread
>> d) if the fix has already been reviewed for inclusion into jdk7u- 
>> dev, the list of reviewers
>> e) once we branch off 7u2, you'll also need to add the forest the  
>> fix is targeted for
>>> I would expect that approval, in principle at least, for a given  
>>> CR needs to be given upfront, and then again once the final code  
>>> change is ready.
>> I think that approval up front + code review should be sufficient.  
>> That may change for phase 2 of a release.
>
> There is a risk with upfront approval and single-reviewer reviews  
> that a change will be committed before being scrutinized by all the  
> right people. (This is migitigated somewhat by the requirement that  
> most changes must be in JDK8 first). Now we want upfront approval-in- 
> principle so that we don't work on CRs that won't meet the criteria  
> for inclusion in an update release, so I'd always seek approval  
> first. But there's no timeline in the review rules to ensure that  
> reviews are open for a minimum amount of time before the change is  
> pushed. One way to fix this is to enforce a minimum review time (say  
> 3 days?); the other is to require a final approval where the  
> approver can insert a delay if they think more time is needed for  
> review feedback to come in. The latter is more work for the approver  
> of course. A combined approach would be a minimum review period with  
> the ability to request an expedited push if needed.

I'd be happy to delay pushes for a few days or have two approvals if  
that's what people want :)  If it turns out that code gets pushed  
without having been reviewed properly, we can take a more strict  
approach as we go forward. It's always a delicate balance between  
adding to much overhead and being to liberal. I think we'll have to  
accept that and fine-tune the process over the coming weeks and months.


>
>>> FYI I have a fix (7039182) that is in 8 and needs to be in 7u2 and  
>>> I'd like to move on this asap, so don't mind being the initial  
>>> tester of the new process, as it were.
>> Sorry for the delay, and thanks for volunteering. Edvard will be  
>> your host while I'm away. ;)
>
> I must have missed the announcement that jdk7u/jdk7u-dev was open  
> for business :) I will commence proceedings immediately.

Feel free :)

Cheers,
Edvard

>
> Cheers,
> David
>
>> cheers,
>> dalibor topic