RFR: 8332313: Update code review guidelines

Phil Race prr at openjdk.org
Wed May 15 20:23:10 UTC 2024


On Wed, 15 May 2024 19:32:04 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> README-code-reviews.md line 103:
>> 
>>> 101: To ensure that new features are consistent with the rest of the API and the desired direction of the Project, a CSR is required for a new Feature, API addition, or behavioral change. The CSR must be reviewed and approved by a "lead" of the Project. Currently this is either Kevin Rushforth or Johan Vos as indicated above.
>>> 102: 
>>> 103: The review of the implementation follows the same "two reviewer" standard for higher-impact changes as described in category B. The two code reviewers for the implementation may or may not include the Lead who reviewed the CSR. The review / approval of the CSR is an independent step from the review / approval of the code change, although they can proceed in parallel.
>> 
>> it is not clear (at least to me) the required order in respect to CSR approval.  for a new feature, does the CSR need to be approved before the work can be started, or the draft PR is published, or an PR enters its "rfr" stage, or just before the integration?
>> 
>> Or perhaps it should be clarified that the CSR is just a human-readable, specially formatted specification of the *change*, and it is an integral part of the new feature that needs to be approved before the feature can be integrated?
>
> This is out of scope for this PR, but it might be something to consider addressing in a follow-on enhancement.

It should be made clear (somewhere, at some point) that it would be rare for a CSR to be completed and approved before starting work on the implementation because the spec. almost always evolves as the fix is developed.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1602196255


More information about the openjfx-dev mailing list