On Thu, 15 Apr 2021 07:53:50 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Process reviewer comments
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> line 1305:
>
>> 1303: // Otherwise, our goal is to leave the index of the cell
>> at the
>> 1304: // top consistent, with the same translation etc.
>> 1305: if (position != 0 && position != 1 && (currentIndex >=
>> cellCount)) {
>
> Comparing a double for equality or inequality is not the best coding
> practice. Anyway, I see this pattern throughout this file. We can live with
> it for now.
The comparison was already in the code. I agree though it is brittle in
general, although it can probably be proved that in this case, the value of
position will never be e.g. 1+\delta with \delta as small as possible.
But it would be good practice for a follow-up issue to remove the equality
checks on doubles in this file (and others).
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/VirtualFlowTest.java
> line 1315:
>
>> 1313: s1 = sb.getLayoutY();
>> 1314: newDelta = newPosition - position;
>> 1315: System.err.println("s0 = "+s0+", s1 = "+s1);
>
> I suggest to comment out all System.err.println() calls from this loop.
> They might be useful while debugging individual failing test, but keeping
> them on for every test run will simply flood the log.
> Similar logs were fixed for stdout earlier. Please refer -
> https://bugs.openjdk.java.net/browse/JDK-8255497
System.err.println()s are removed now.
-------------
PR: https://git.openjdk.java.net/jfx/pull/398
