On Wed, 4 Jun 2025 18:37:02 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Khalid Boulanouare has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removes unnecessary lines and keep consistent code format
>
> test/jdk/java/awt/Frame/MultiScreenTest.java line 1:
>
>> 1: /*
>
> throw new SkippedException("You have only one monitor in your system - test
> passed");
>
> Please remove `" - test passed"`, the test is skipped.
message updated.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 85:
>
>> 83: "Actively drag the DitherTest frames on the secondary
>> screen and " +
>> 84: "if you see garbage appearing on your primary screen " +
>> 85: "test failed otherwise it passed.";;
>
> Suggestion:
>
> "test failed otherwise it passed.";
semicolon removed.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 103:
>
>> 101: JFrame f = new JFrame(gc[i]); // test JFrame( gc )
>> 102: GCCanvas c = new GCCanvas(gc[i]); // test canvas(
>> gc )
>> 103: Rectangle gcBounds = gc[i].getBounds(); // test
>> getBounds()
>
> I suggest removing the comments altogether — they don't add anything to
> what's in the code.
> Suggestion:
>
> JFrame f = new JFrame(gc[i]);
> GCCanvas c = new GCCanvas(gc[i]);
> Rectangle gcBounds = gc[i].getBounds();
comments removed.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 108:
>
>> 106:
>> 107: f.getContentPane().add(c);
>> 108: f.setTitle("Screen# " + Integer.toString(j) + ",
>> GC#" + Integer.toString(i));
>
> Can be simplified:
> Suggestion:
>
> f.setTitle("Screen# " + j + ", GC#" + i);
code updated.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 111:
>
>> 109: f.setSize(300, 200);
>> 110: f.setLocation(400 + xoffs, (i * 150) + yoffs); //
>> test
>> 111: // displaying in right location
>
> Suggestion:
>
> // test displaying in right location
> f.setLocation(400 + xoffs, (i * 150) + yoffs);
>
> The comment applies to this line, however, does it add anything, does it
> clarify anything? Should we remove the comment?
Comment cleanup done, pending decision about its removal.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 114:
>
>> 112: list.add(f);
>> 113:
>> 114: Frame ditherfs = new Frame("DitherTest GC#" +
>> Integer.toString(i), gc[i]);
>
> Can be simplified:
> Suggestion:
>
> Frame ditherfs = new Frame("DitherTest GC#" + i, gc[i]);
code updated.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 115:
>
>> 113:
>> 114: Frame ditherfs = new Frame("DitherTest GC#" +
>> Integer.toString(i), gc[i]);
>> 115: ditherfs.setLayout(new BorderLayout()); //
>> showDitherTest
>
> The comment is misleading: setting a layout doesn't show anything.
> Suggestion:
>
> ditherfs.setLayout(new BorderLayout());
comment removed.
> test/jdk/java/awt/Frame/MultiScreenTest.java line 140:
>
>> 138: super(gc);
>> 139: this.gc = gc;
>> 140: bounds = gc.getBounds();
>
> Remove
>
> Graphics g = this.getGraphics();
>
> This field is unused.
line removed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128224893
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128227891
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128228950
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128232394
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128231557
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128232848
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128222035
PR Review Comment: https://git.openjdk.org/jdk/pull/24752#discussion_r2128226551
