Draft Public Code Review Proposal (original) (raw)

Edvard Wendelin edvard.wendelin at oracle.com
Wed Aug 3 00:30:02 PDT 2011

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

More information about the jdk7u-dev mailing list