RFR: 8280818: Expand bug8033699.java to iterate over all LaFs
Alexey Ivanov
aivanov at openjdk.org
Wed Mar 12 17:49:08 UTC 2025
On Tue, 11 Mar 2025 22:52:39 GMT, Damon Nguyen <dnguyen at openjdk.org> wrote:
>> Should these be done as part of this change or a separate bug for refactoring /code cleanup of this test ?
>
> I usually fix these when I touch the test anyway.
> mainFrame = new JFrame("Bug 8033699 - 9 Tests for Grouped / Non-Grouped Radio Buttons");
Makes sense… However, a generic title would be good enough. Something like _“Radio button focus tests”_. The current title is too long, it doesn't fit in the title bar of the frame (at least on Windows), therefore I see no point in making it comprehensive and long.
> I usually fix these when I touch the test anyway.
In majority of cases, I do too. Yet I tend not to change lines that I don't touch. From this point of view, additional changes aren't necessary — none of the lines that don't fit into 80-column limit aren't touched.
The problem I see with additional refactoring is that it adds noise to the code review and it makes it harder to understand what the real, important changes are.
Use *your common sense*.
> Please limit to 80 cols wherever applicable
This is not applied strictly… I'm for following the 80-column limit where it doesn't reduce the readability. Yet I'm for stronger enforcement of 100-column limit. There are quite a few lines which are longer than 100 columns. The culprit is `KeyboardFocusManager.getCurrentKeyboardFocusManager().getFocusOwner()` which accounts for 70 characters.
I'd like to make it shorter, and the focus manager can be cached after the first usage. At the same time, I'm unsure doing so in this code review is reasonable.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23964#discussion_r1992004341
More information about the client-libs-dev
mailing list