On Mon, 12 Oct 2026 15:59:14 GMT, Marius Hanl <[email protected]> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove redundant invalidation note (covered by super interface)
>
> modules/javafx.base/src/test/java/test/com/sun/javafx/binding/AbstractListenerManagerTest.java
> line 185:
>
>> 183: ChangeListener<String> cl2 = (_, o, n) ->
>> notifications.add("CL2: " + o + " -> " + n);
>> 184:
>> 185: helper.addListener(ov, (ChangeListener<String>) (_, o, n) -> {
>
> This cast isn't needed. (and most of the time below as well. Sometimes it's
> needed).
Removed everywhere it wasn't needed.
> modules/javafx.base/src/test/java/test/com/sun/javafx/binding/ListenerListBaseTest.java
> line 67:
>
>> 65: }
>> 66:
>> 67: public boolean accessibleUnlock() {
>
> the return type here is unused, so I would prefer making it `void` in the test
I will leave that as it is.
> modules/javafx.base/src/test/java/test/com/sun/javafx/binding/ListenerListTestBase.java
> line 696:
>
>> 694:
>> 695: private Pattern PATTERN = Pattern.compile("(?<listener>.*): changed
>> from (?<old>.*) to (?<new>.*)");
>> 696: private Pattern INVALIDATION_PATTERN =
>> Pattern.compile("(?<listener>.*): current value (?<current>.*)");
>
> I would prefer those at the start of the class (like the other attributes),
> but minor.
> (Also both could be static final, variable is written in uppercase, so this
> may was your intention?)
Yeah, it looks like this wasn't cleaned up properly, moved to top and made
`static final` which is indeed no doubt what I had intended all along.
> modules/javafx.base/src/test/java/test/javafx/beans/ObservableValueTest.java
> line 385:
>
>> 383: _ -> valueSetter.accept(action.getValue().equals(value1) ?
>> value2 : value1),
>> 384: calls,
>> 385: new int[] {50, 25, 13, 6, 3, 2, 1, 1, 1} // won't reach 0
>> as a new listener is added each time
>
> `new int[]` can be removed here and below
I'm not sure that improves readability in this case.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r4251839144
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r4251832588
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r4251827703
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r4251816091
