RFR: 8332313: Update code review guidelines

[John Hendrikx]

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

>> Yeah, that was a typo (which I didn't notice when copying the block from the other doc). I'll fix it. And I agree with your concern, so I'll remove the last sentence.
>
> I agree with the concern, but I still think it's much better to encourage developers to do formatting in a separate issue (or not at all) with all the required administration, than to sneak in a formatting change in a PR that has nothing to do with formatting. I prefer the noise to be completely isolated, so that it can be ignored easily, rather than being distracted by it in a PR.

I think this conflicts a bit with the previous statement ("don't worry too much about import order"). In my experience, when merging complex changes, import order changes are often the cause of conflicts. We also found that with many different committer preferences, the whole import block could change from one commit to the next, which will lead to a conflict if such a file is part of a backport.

We solved this by simply mandating a single order for imports (alphabetical, no gaps, no star imports) and made the build fail when it was violated.  The number of conflicts went down dramatically.  This does require a one time import fix of the code base, which will also be a source of conflicts, but at least it will be in the usual area (imports) and should solve the problem once and for all.

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

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