Hello Semyon,

Thank you, that eliminates the question with the internal package. Probably Brian meant the same (but I just didn't understand this at first).

Please review the updated webrev:
http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/

Thanks,
Alexander

On 1/11/2016 11:59 AM, Semyon Sadetsky wrote:
Hello Alexander,

Why don't use the reader method analogue for writer? Then the test could use the public API which is preferable.

Just a cosmetic note. Lines should be wrapped at 80 position.

--Semyon

On 12/29/2015 3:44 PM, Alexander Stepanov wrote:
Hello Brian,

Thank you for the notes, please see the updated webrev:
http://cr.openjdk.java.net/~yan/8145776/webrev.01/

1., 3. - fixed
WRT 2.: this import is necessary to use TIFFImageWriter, TIFFImageWriterSpi. As it follows from http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988, the majority of new classes (excepting tag sets) were added to com.sun.imageio.plugins.tiff. Do you mean these classes would be moved to javax/imageio soon? (please note also a @module tag added for compatibility with modular java).

Thanks,
Alexander

On 12/29/2015 2:12 AM, Brian Burkhalter wrote:
Hello Alexander,

A few comments.

1) Lines 53,59: By convention it is better to put constants in upper case, e.g., NUM_IMAGES and BLACK_SIZE.

2) Line 45, 102: The test references the internal package com.sun.imageio.plugins.tiff. In general it is better to use the public packages.

3) Line 69: If the test fails, it would be good to know the random seed. It would also be good to be able to set the seed when running the test. The approach taken in core libraries may be seen in test/java/math/BigInteger/BigIntegerTest.java. The jdk.testlibrary.RandomFactory class is used to create the Random instance and to obtain any seed provided via Java properties. This requires the test tags “@library /lib/testlibrary” and “@build jdk.testlibrary.*” and it is also good to add “@key randomness.”

Regards,

Brian

On Dec 25, 2015, at 5:10 AM, Alexander Stepanov <alexander.v.stepa...@oracle.com> wrote:

Could you please review the following fix
http://cr.openjdk.java.net/~yan/8145776/webrev.00/ <http://cr.openjdk.java.net/%7Eyan/8145776/webrev.00/>
for
https://bugs.openjdk.java.net/browse/JDK-8145776

Just a single test added checking the correctness of multi-page TIFF image creation.




Reply via email to