RFR: 8332313: Update code review guidelines
John Hendrikx
jhendrikx at openjdk.org
Wed May 15 20:18:11 UTC 2024
On Wed, 15 May 2024 17:45:56 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
> Update the code review guidelines for JavaFX.
>
> The JavaFX [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md) guidelines includes guidance for creating, reviewing, and integrating changes to JavaFX, along with a pointer to a [Code Review Policies](https://wiki.openjdk.org/display/OpenJFX/Code+Reviews) Wiki page.
>
> This PR updates these guidelines to improve the quality of reviews, with a goal of improving JavaFX and decreasing the chance of introducing a serious regression or other critical bug.
>
> The source branch has three commits:
> 1. Converts the Code Review Policies Wiki page to a [README-code-reviews.md](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/README-code-reviews.md) file in the repo and updates hyperlinks to the new location.
> 2. Update `README-code-reviews.md` with new guidelines
> 3. Update `CONTRIBUTING.md` to highlight important requirements (and minor changes to `README-code-reviews.md`)
>
> Commit 1 is content neutral, so it might be helpful for reviewers to look at the changes starting from the second commit.
>
> The updates are:
>
> * In the Overview section, add a list of items for Reviewers, PR authors, and sponsoring Committers to verify prior to integration
> * Create a "Guidelines for reviewing a PR" subsection of the Code review policies section
> * Create a "Before you integrate or sponsor a PR" subsection of the Code review policies section
> * Update the `CONTRIBUTING.md` page to highlight important requirements
Marked as reviewed by jhendrikx (Committer).
README-code-reviews.md line 79:
> 77: * All Reviewers who have requested the chance to review have done so (or indicated that they are OK with it going in without their review). In rare cases a Project Lead may override this.
> 78: * The PR has been "rfr" (as indicated by Skara) for at least 1 business day (24 hours), not including weekends or major holidays. This is to allow sufficient time for those reviewers who might be in other time zones the chance to review if they have concerns. This is measured from the time that Skara has most recently added the "rfr" label (for example, for a PR that was previously in Draft mode, wait for at least 24 hours after the PR has been taken out of Draft and marked "rfr"). In rare cases (e.g., a build breakage) a Reviewer might give the OK to integrate without waiting for 24 hours.
> 79: * Verify the commit message. The Skara tooling adds a comment near the top of the PR with the commit message that will be used. You can add a summary to the commit message with the `/summary` command. You can add additional contributors with the `/contributor` command. Commands are issued by adding a comment to the PR that starts with a slash `/` character.
The last sentence feels a bit out of place, as several places earlier in this document already mention slash commands.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1455#pullrequestreview-2058908894
PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1602186633
More information about the openjfx-dev
mailing list