On Thu, 16 May 2024 07:27:34 GMT, Johan Vos <j...@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
