[OpenJDK 2D-Dev] Request for review: 7196571, 7196572, 7196573: javac warnings cleanup from Adopt OpenJDK bugathon

[Phil Race]

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