On Fri, 11 Apr 2025 10:31:14 GMT, Mikhail Yankelevich
<myankelev...@openjdk.org> wrote:
>> Jayathirth D V has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update after review
>
> test/jdk/javax/swing/JList/bug4618767.java line 29:
>
>> 27: * @summary First letter navigation in JList interferes with mnemonics
>> 28: * @key headful
>> 29: * @run main bug4618767
>
> Nitpick: is this line necessary?
Test will run without this line also.
I just follow it as standard template. Will leave it as it is.
> test/jdk/javax/swing/JList/bug4618767.java line 55:
>
>> 53: public static void main(String[] args) throws Exception {
>> 54: try {
>> 55: listGainedFocusLatch = new CountDownLatch(1);
>
> Do you think it might be better to initialise it directly on line 52?
>
>
> private static CountDownLatch listGainedFocusLatch = new CountDownLatch(1);
Better to do it.
Updated.
> test/jdk/javax/swing/JList/bug4618767.java line 86:
>
>> 84: });
>> 85:
>> 86: list = new JList(new String[] {"one", "two", "three",
>> "four"});
>
> Do you think it might be easier to read if this is initialised on line 49?
>
>
> private static final JList list = new JList(new String[] {"one", "two",
> "three", "four"});
>
>
> and the same for line 69 `f = new JFrame("bug4618767");`
>
> I feel it is going to be easier to read if global variables were initialised
> at class level when possible, especially if they are not reassigned further.
> This way they can also be finalised. But if you feel it's ok as it is, I'm
> fine with it.
We can initialise JList early.
JFrame related one will keep in same invokeAndWait()
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24588#discussion_r2041458855
PR Review Comment: https://git.openjdk.org/jdk/pull/24588#discussion_r2041459538
PR Review Comment: https://git.openjdk.org/jdk/pull/24588#discussion_r2041460746
