RFR: 8332313: Update code review guidelines [v2]

[Kevin Rushforth]

On Thu, 16 May 2024 07:27:34 GMT, Johan Vos <jvos at openjdk.org> wrote:

>> That's a good idea.
>
> In the ideal world where we have tons of regression and compatibility tests, I would agree. Unfortunately, we are totally not there yet. Compared to other projects, the quality of tests in OpenJFX is good, but given the multitude of usage scenario's in client development, we would need much, much more tests before we can rely on them to detect regression.
> Therefore, the #1 point imho is that both the author (as Andy commented) and the reviewer have a real good understanding on what is happening. Every single change in a PR should be understood. The most dangerous things are "I don't understand why, but it seems to fail before and succeed after the patch".

Yes, this a good point. I left the "make sure you understand..." bullet as the first, and moved the "Focus first on substantive comments" below the regression and compatibility bullets.

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

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