RFR: 8332313: Update code review guidelines [v2]
Kevin Rushforth
kcr at openjdk.org
Fri May 17 14:10:43 UTC 2024
> 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
Kevin Rushforth has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 20 additional commits since the last revision:
- Wait for re-review if you've pushed a change addressing a substantive comment
- Typo + remove sentence that invites reformatting PRs
- Wording changes, added example of API addition
- Formatting
- Add item about checking the target branch
- Remove trailing period from previous
- Minor changes regarding syncing from upstream master
- Clarify that GHA tests might fail for a reason unrelated to the PR
- Fix typo
"but It" --> "but it"
- Remove bullet about checking the commit message (Skara already checks)
- ... and 10 more: https://git.openjdk.org/jfx/compare/3f56e6c0...9cf7f920
-------------
Changes:
- all: https://git.openjdk.org/jfx/pull/1455/files
- new: https://git.openjdk.org/jfx/pull/1455/files/1de03ec7..9cf7f920
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jfx&pr=1455&range=01
- incr: https://webrevs.openjdk.org/?repo=jfx&pr=1455&range=00-01
Stats: 51497 lines in 127 files changed: 26323 ins; 14206 del; 10968 mod
Patch: https://git.openjdk.org/jfx/pull/1455.diff
Fetch: git fetch https://git.openjdk.org/jfx.git pull/1455/head:pull/1455
PR: https://git.openjdk.org/jfx/pull/1455
More information about the openjfx-dev
mailing list