On Wed, 15 May 2024 20:08:05 GMT, John Hendrikx <jhendr...@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 > > 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. I'll remove it (I moved it from the CONTRIBUTING doc, and it does seem out of place). Actually as someone else commented, Skara will verify that the PR title and JBS bug match, so mostly this is about adding a summary and/or contributors if you want to. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1602315921
