Draft Public Code Review Proposal (original) (raw)

Dalibor Topic dalibor.topic at oracle.com
Tue Aug 2 20:09:03 PDT 2011

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

More information about the jdk7u-dev mailing list