On Tue, 24 Dec 2024 03:02:51 GMT, anass baya <d...@openjdk.org> wrote:
> Remove PassFailJFrame constructor with screenshots since no test is using it
Changes requested by aivanov (Reviewer).
Nearly good.
Please, also update the copyright year at the top of the file: 2024 → 2025.
Looks good.
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 407:
> 405: * the default values of {@value #ROWS} and {@value #COLUMNS}
> 406: * for rows and columns.
> 407: * The screenshot feature is not enabled, if you use this
> constructor.
I think this line about the screenshot should be removed now.
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 458:
> 456: invokeAndWait(() -> createUI(title, instructions,
> testTimeOut,
> 457: rows, columns));
> 458: }
Please, keep the existing call to `invokeOnEDT`, just remove the last parameter
from `createUI`:
Suggestion:
invokeOnEDT(() -> createUI(title, instructions,
testTimeOut,
rows, columns));
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 473:
> 471: * @see Builder Builder
> 472: */
> 473:
This blank line is redundant, follow the code style in the file where there's
no blank line between the javadoc and the declaration.
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 482:
> 480: rows, columns));
> 481: }
> 482:
No trailing whitespace is allowed. The `jcheck` bot has marked it as an error
and blocker.
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 510:
> 508: builder.positionWindows
> 509:
> .positionTestWindows(unmodifiableList(builder.testWindows),
> 510:
> builder.instructionUIHandler));
Move the documentation to the constructor above.
https://github.com/openjdk/jdk/blob/26b8b1e71f231d4601737a295dfed1ffd39f9b42/test/jdk/java/awt/regtesthelpers/PassFailJFrame.java#L449-L451
Otherwise, you're removing the detailed documentation, which other
constructors refer to. You may need to amend some sentences.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2534741147
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2537104573
Marked as reviewed by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22873#pullrequestreview-2537962780
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907158578
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1905703607
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907165044
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1907165979
PR Review Comment: https://git.openjdk.org/jdk/pull/22873#discussion_r1905688661
