RFR: 8332313: Update code review guidelines [v2]

Nir Lisker nlisker at openjdk.org
Fri May 17 15:12:13 UTC 2024


On Fri, 17 May 2024 14:58:41 GMT, Nir Lisker <nlisker 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/1b088e5b...9cf7f920
>
> README-code-reviews.md line 72:
> 
>> 70: * Focus first on substantive comments rather than stylistic comments
>> 71: * Check whether there is an automated test; if not, ask for one, if it is feasible
>> 72: * Make sure that the PR has executed the GitHub Actions (GHA) tests; if they aren't being run, ask the PR author to enable GHA workflows; if the test fails on some platforms, check whether it is a real bug (sometimes a job fails becau se of GHA infrastucture changes or we see a spurious GHA failure)
> 
> becau se -> because

Never mind, got simul-fixed.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605160705


More information about the openjfx-dev mailing list