I reviewed all the changes and they look fine to me with just formatting
issues
and one code change question or suggestion
-* an identation issue at line 596 here
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.imageio/src/share/classes/com/sun/imageio/plugins/common/ImageUtil.java.sdiff.html
The removal of the cast means the other lines aren't aligned any more
* I also wonder why lines 837 & 838 can't be collapsed into one line ?
0x00000001 << (31 - bOffset % 32));
* By contrast lines 1035 and 1115 here are now much, much > 80 chars
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.imageio/src/share/classes/javax/imageio/ImageIO.java.sdiff.html
Also for both of these cases, you added .asSubClass(..).
Now when I look at the new code there, it looks a lot busier, all to
avoid one
simple (ImageWriterSpi) cast. Is it really worth it ?
And if the change is made, maybe you should check for ClassCastException.
I guess we already vulnerable to that, although its rather unlikely to
occur ...
I double checked for most of the serial verson uids in the imaging &
printing classes
that they were generated from running serialver on the classes so as to
maintain
the existent values, so that looks correct.
Don't forget you need a second reviewer for all the changes.
-phil.
On 9/11/2012 11:40 AM, Martijn Verburg wrote:
Hi all,
Artem Ananiev very kindly raised bugs and a webrev for the patches
sent in from a Bugathon we ran back in April (patches have been tested
against latest source tree).
The bug numbers are:
7196571: [Bugathon] Reduce the number of javac warnings in ImageIO
7196572: [Bugathon] Reduce the number of javac warnings in color management
7196573: [Bugathon] Reduce the number of javac warnings in imaging
The corresponding webrevs are at:
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.imageio/
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-color/
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-image/
http://cr.openjdk.java.net/~art/Bugathon-2012/webrev.j2d-print/
Thanks to Artem, Stuart and Phil for helping me navigate through the
AWT/2D waters :-)
Cheers,
Martijn
