On Wed, 15 May 2024 17:45:56 GMT, Kevin Rushforth <k...@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

Reply via email to