On Tue, 13 Feb 2024 12:06:25 GMT, vtstydev <[email protected]> wrote:
>> More correct way to take in consideration nonzero PHYSICALOFFSETX,
>> PHYSICALOFFSETY of device for banded-raster printing loop. Only on Windows
>> platform under certain conditions real device prints shifted image on paper.
>
> vtstydev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Done requested fixes 5
The copyright must contain two years at most, you modify the second one to be
the current year. With the three years, it will break the build because of
incorrect copyright header.
src/java.desktop/share/classes/sun/print/RasterPrinterJob.java line 2:
> 1: /*
> 2: * Copyright (c) 1998, 2023, 2024, Oracle and/or its affiliates. All
> rights reserved.
Suggestion:
* Copyright (c) 1998, 2024, Oracle and/or its affiliates. All rights reserved.
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
60:
> 58: "Tested bug occurs only on-paper printing " +
> 59: "so you mustn't use PDF printer\n\n" +
> 60: "1. Java print dialog should appear.\n" +
Suggestion:
"Tested bug occurs only on-paper printing " +
"so you mustn't use PDF printer\n\n" +
"1. Java print dialog should appear.\n" +
Alternatively, reduce the indentation of *all* the lines with the instructions.
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
86:
> 84: }
> 85: PassFailJFrame.builder().instructions(instructionsHeader +
> INSTRUCTIONS)
> 86: .rows(15).testUI(() ->
> createTestUI()).build().awaitAndCheck();
Suggestion:
PassFailJFrame.builder()
.instructions(instructionsHeader + INSTRUCTIONS)
.rows(15)
.testUI(() -> createTestUI())
.build()
.awaitAndCheck();
What do you think? Is it easier to read?
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
92:
> 90: "Printer not configured or available.");
> 91: throw new RuntimeException("Test failed : " +
> 92: "Printer not configured or available.");
I'd rather keep those unmodified, the string is not so long, and wrapping it
doesn't improve readability.
In my opinion, `println` is redundant, the runtime exception conveys it.
test/jdk/java/awt/print/PrinterJob/ImagePrinting/AlphaPrintingOffsets.java line
138:
> 136: printerJob.setPageable(bookPageSavingTest);
> 137: }
> 138: else {
Suggestion:
} else {
This is the Java style and you follow this style in other places.
-------------
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17030#pullrequestreview-1878115023
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1487968754
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1487975023
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1487988493
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1487980225
PR Review Comment: https://git.openjdk.org/jdk/pull/17030#discussion_r1487986622
