RFR: 8332313: Update code review guidelines [v2]

[Kevin Rushforth]

On Wed, 15 May 2024 18:30:06 GMT, Andy Goryachev <angorya at 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