On Fri, 25 Apr 2025 16:55:40 GMT, Rajat Mahajan <rmaha...@openjdk.org> wrote:

>> Details:
>> Refactored code as requested in the Bug description.
>> 
>> Tested and verified the test passes.
>
> Rajat Mahajan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   code changes as per code review

Looks good to me, except for minor nits.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 62:

> 60:     public static void main(String[] args) throws Throwable {
> 61:         robot = new Robot();
> 62:         SwingUtilities.invokeAndWait(() -> {

I wonder why you removed the blank line, I'm strongly for restoring it here.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 64:

> 62:         SwingUtilities.invokeAndWait(() -> {
> 63:             focusManager = 
> KeyboardFocusManager.getCurrentKeyboardFocusManager();
> 64:         });

Suggestion:

        SwingUtilities.invokeAndWait(() ->
                focusManager = 
KeyboardFocusManager.getCurrentKeyboardFocusManager());

Braces are redundant for one statement (in lambdas), or _“Statement lambda can 
be replaced with expression lambda.”_

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 66:

> 64:         });
> 65:         // Get all installed Look and Feels
> 66:         UIManager.LookAndFeelInfo[] lafs = 
> UIManager.getInstalledLookAndFeels();

Suggestion:

            focusManager = 
KeyboardFocusManager.getCurrentKeyboardFocusManager();
        });

        UIManager.LookAndFeelInfo[] lafs = UIManager.getInstalledLookAndFeels();

I'd add a blank here, too, and remove the comment since 
`getInstalledLookAndFeels` is already self-documenting.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 170:

> 168: 
> 169:         // Create main vertical box
> 170:         Box mainBox = Box.createVerticalBox();

Suggestion:

        Box mainBox = Box.createVerticalBox();

The comment is redundant, it doesn't add anything to what one could infer from 
the statement.

test/jdk/javax/swing/JRadioButton/8033699/bug8033699.java line 176:

> 174:         mainBox.add(btnEnd);
> 175: 
> 176:         mainFrame.getContentPane().add(mainBox);

Suggestion:

        mainFrame.add(mainBox);

At this point, @honkar-jdk's can be applied. I'll leave it to your discretion, 
either way is fine with me.

-------------

Marked as reviewed by aivanov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/24384#pullrequestreview-2795116203
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060786756
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060798663
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060790607
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060792449
PR Review Comment: https://git.openjdk.org/jdk/pull/24384#discussion_r2060794078

Reply via email to