On Fri, 5 May 2023 18:03:12 GMT, Phil Race <p...@openjdk.org> wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comment update
>
> test/jdk/javax/swing/JMenuItem/bug4304129.java line 47:
>
>> 45: public static void main(String args[]) throws Exception {
>> 46: bug4304129 test = new bug4304129();
>> 47: test.init();
>
> I don't see why we need an instance here.
> The body of init() can be moved into main() and mnemonic and accelerator made
> static
Updated.
> test/jdk/javax/swing/JMenuItem/bug4839464.java line 79:
>
>> 77: public static JButton changeShortDescButton;
>> 78:
>> 79: public JMenuItem item;
>
> it seems a bit odd that this and some below are instance vars and the ones
> above are static
>
> Should you make them all static ?.. and then you don't need an instance of
> the test class either
> Or make them all instance .. either way ...
>
> I also find it odd that this test creates TWO Robots - one AWT Robot and one
> JRobot.
> Can you consolidate these ?
Updated.
> test/jdk/javax/swing/JMenuItem/bug4966168.java line 46:
>
>> 44: public static void main(String args[]) throws Exception {
>> 45: bug4966168 test = new bug4966168();
>> 46: test.init();
>
> No need I can see for the instance. Collapse init() into main()
Updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232377
PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232672
PR Review Comment: https://git.openjdk.org/jdk/pull/13822#discussion_r1187232953
