On Wed, 15 May 2024 19:16:31 GMT, Nir Lisker <nlis...@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 60: > >> 58: * If you want an area expert to review a PR, indicate this in a comment >> of the form: `Reviewers: @PERSON1 @PERSON2`; the requested reviewers can >> indicate whether or not they plan to review it >> 59: * If you want to ensure that you have the opportunity to review this PR >> yourself, add a comment of the form: `@PRAUTHOR Wait for me to review this >> PR`, optionally add any concerns you might have >> 60: > > I would add that it's a good idea to search for JBS issues similar/linked to > the one being targeted. There are many unfound duplicates in JBS that arise > only when searching, or at least closely linked ones (problems in > `TableTreeView` often have "pseudo-duplicates" for `TreeView` and > `TableView`, for example). > > I would explain (or link to where it is explained) when to close an issue a a > duplicated vs. when to use the /add Skara command to bundle issues into the > same fix. > > We might also find that the targeted issue is a regression of another issue. > This is somewhat mentioned in the bullet point below "Consider the risk of > regression". Second this, although I think the bulk of the work should fall on the author of the PR. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1602168856
