Draft Public Code Review Proposal

[Dalibor Topic]

On 7/14/11 11:50 PM, Dr Andrew John Hughes wrote:
> On 03:45 Thu 14 Jul     , Dalibor Topic wrote:
> 
>> Next item on my draft list is code review. The proposal below is
>   inspired by the developer guide section on code review, and in part
>   inspired by the processes in OpenJDK 6. It tries to encourage
>   transparency, and getting more eyes on the code, while at the same
>   time allowing established practices, like asking component leads for
>   additional reviews, to continue to work.
> 
>> Public code review:
> 
>>  Preamble: The new infrastructure is not ready yet, so we won't
>> switch to it for the next release in OpenJDK (i.e. 7u2, as 7u1 is a
>> CPU) just yet. The process here may change for 7u4.
> 
> What is this 'CPU'?  You refer to it both here and in the last
> document, but I assume it's not Central Processing Unit.
> 
>> Pre-requisite: Before asking for code review, make sure your fix is
>   applied on the most recent revision of the code you intend to work
>   on, that there are no build errors on the supported platforms for
>   the release you're working on, and that any included or otherwise
>   applicable tests pass without failures on all relevant platforms. If
>   you're not able to build or test on all supported platforms, you
>   MUST let the reviewers know which ones you were able to build and
>   test on, as well as whether you anticipate any issues on the
>   platforms you haven't been able to build and test on.
>>
> 
> These rules are very stringent.  I doubt anyone outside Oracle is going
> to be able to build and test on all platforms.  Running the full test suite
> on any platform is non-trivial and something that should be provided by
> mainline infrastructure, not something every developer has to set up.

Yeah, I agree - that's why there is the escape hatch of letting the 
reviewers know that you couldn't run it on some platforms.

>> Rule 0: Code reviews for public JDK 7 Update forests MUST be done
>> publicly: Either through e-mail on jdk7u-dev at openjdk.java.net
>> mailing list, or using some other suitable public mechanism. If a
>> code review is not done on the jdk7u-dev at openjdk.java.net mailing
>> list, as part of the approval request for inclusion of the fix into
>> a public JDK 7 Updates forest, which you MUST send to the
>> jdk7u-dev at openjdk.java.net mailing list, a URL for the public code
>> review MUST be provided.
> 
>> Rule 1: If the content of a changeset submitted for review for a
>> public JDK 7 Update forest is the same as the corresponding
>> changeset submitted or committed for JDK 8, then it does not have to
>> be resubmitted. In that case its URL will suffice.
> 
>> Rule 2: If the content of a changeset submitted for review for a public JDK 7 Update forest differs from the corresponding JDK 8 changeset, or if there is no corresponding JDK 8 changeset, then the changeset MUST be submitted publicly for review:
>>
>>     * A very small changeset MAY be submitted as a simple patch, as described in http://openjdk.java.net/contribute/
>>
>>     * Other than that, changesets SHOULD be submitted as webrevs on the code review server, cr.openjdk.java.net.
>>
>>     * Alternatively, a URL for the webrev of the changeset MAY be provided
>>
> 
> What is the reasoning behind continuing to use webrevs?  They separate the patch
> from the discussion and there seems to be no guarantee that they won't just disappear,
> as they aren't archived with the mails.

It's the currently ubiquitous low level option available to anyone inside and outside
Oracle. When we move on to use another/a real code review system, then I expect the 
code review rules to be rewritten to take it into account.

cheers,
dalibor topic
-- 
Oracle <http://www.oracle.com>
Dalibor Topic | Java F/OSS Ambassador
Phone: +494023646738 <tel:+494023646738> | Mobile: +491772664192 <tel:+491772664192>
Oracle Java Platform Group

ORACLE Deutschland B.V. & Co. KG | Nagelsweg 55 | 20097 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Jürgen Kunz, Marcel van de Molen, Alexander van der Ven

Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment