On Wed, 15 May 2024 18:30:06 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> 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 > > README-code-reviews.md line 78: > >> 76: * All substantive feedback has been addressed, especially any objections >> from one with a Reviewer role. >> 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. > > nit pick: 24 hours does not always equal to one business day. A long weekend > may extend the waiting period to 96 hours, a "winter break" might span the > whole week. fixed > 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: * Double-check the commit message before you integrate. Skara will list >> it near the top of your PR. > > is this needed? SKARA makes sure that it matches the JBS title. removed bullet ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605060203 PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605060466
