Draft Public Code Review Proposal

[Kelly O'Hair]

On Jul 13, 2011, at 6:45 PM, 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.
> 
> 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.
> 
> 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.

I'm having problems parsing the above sentences. Is it just me?
I think I understand what it's basically saying though.

> 
> 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.

I have a little issue with this. If it's in jdk8, then you can't just pull the exact same changeset into jdk7u,
you can import the same patch and create a new changeset with the same patch, but the patch will be likely
applied to different sources, so it's questionable in my mind that it can be called 'the same'.
Perhaps the words 'same patch' here?

> 
> 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
> 
> Rule 3: A changeset submitted for the JDK 7 Update mainline forest (jdk7u) MUST be reviewed by at least one reviewer. The maintainer responsible for approval of the changeset MAY request additional, specific reviewers to review the changeset, e. g. component leads.
> 
> Rule 4: A changeset submitted for a JDK 7 Update release forest (like jdk7u2, once it's branched off) MUST be reviewed by at least two reviewers. The maintainer responsible for approval of the changeset MAY request additional, specific reviewers to review the changeset, e. g. component leads.

Should we require that all changesets applied to the jdk7uN forests, also be applied to the jdk7u forest?

-kto

> 
> 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