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