Hi All, I happy to see another RC come along! :) This is large code base so I appreciate that the reports generate a lot of potential work.
-1: Unapproved license: src/main/java/org/apache/commons/imaging/formats/jpeg/segments/App14Segment.java Fix obvious FindBugs (to me only?): - Possible NPE with NP_IMMEDIATE_DEREFERENCE_OF_READLINE<http://findbugs.sourceforge.net/bugDescriptions.html#NP_IMMEDIATE_DEREFERENCE_OF_READLINE>@ https://people.apache.org/~damjan/imaging-1.0rc4/xref/org/apache/commons/imaging/formats/rgbe/RgbeInfo.html#103 - Is this useless or an incomplete impl?: Useless control flow in org.apache.commons.imaging.palette.PaletteFactory.makePaletteFancy(BufferedImage) STYLE UCF_USELESS_CONTROL_FLOW<http://findbugs.sourceforge.net/bugDescriptions.html#UCF_USELESS_CONTROL_FLOW> 54<https://people.apache.org/%7Edamjan/imaging-1.0rc4/xref/org/apache/commons/imaging/palette/PaletteFactory.html#54> - If we want non-standard class name (starts with a lower case, then Javadoc would be nice to understand why this is justified: public static class tEXt extends PngText public static class zTXt extends PngText public static class iTXt extends PngText - What about the RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE<http://findbugs.sourceforge.net/bugDescriptions.html#RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE>? - What about the FE_FLOATING_POINT_EQUALITY<http://findbugs.sourceforge.net/bugDescriptions.html#FE_FLOATING_POINT_EQUALITY>? - IIRC, the EI_EXPOSE_REP<http://findbugs.sourceforge.net/bugDescriptions.html#EI_EXPOSE_REP>issues are just a normal by-product of the design? Please confirm. - PMD - do the easy clean ups of "Unnecessary final modifier in final class". - "Avoid empty catch blocks": Add a comment as to why the block is empty, for example "This exception cannot be thrown because...". This is important for anyone who tries to grok the code. - "Avoid unused method/fields/etc": Is this unimplemented code or cruft? If cruft, it should be removed before we give ourselves unneeded BC headaches. - CDP: lots of those but I do not know the code base enough to tell how easy it would be to refactor duplications and if refactoring is justified. - Code coverage: overall, is not great. These packages have 0%.code coverage. - org.apache.commons.imaging.formats.transparencyfilters - org.apache.commons.imaging.formats.psd.dataparsers - org.apache.commons.imaging.formats.psd.datareaders - org.apache.commons.imaging.icc Oddly org.apache.commons.imaging.util has only 17% which I would think would be higher since it is a util package. - Process This might be a case of RM process but when I RM I set the release date in changes.xml to the date I cut the RC. If the RC passes, then you are done, otherwise you edit it again for the next RC. - Javadoc Only 3 out of ~40 packages have comments, which makes it harder to find your way around. - Checkstyle is not strict enough - IMO should always complain about missing { } in blocks. There also a lot of generics warnings (in Eclipse, any IDE will tell you these days.) Sorry to make it a long laundry list, but there is only one 1.0 ;) There is probably more of course, but I have to go now... Gary On Tue, Sep 25, 2012 at 12:33 AM, Damjan Jovanovic <damjan....@gmail.com>wrote: > Please vote on releasing commons-imaging 1.0 from RC4. > > A number of things were changed from RC2: dead local store warnings in > Findbugs were fixed, missing ASL headers were added to Javadoc files, > developers and contributors were populated into pom.xml, a few > last-minute bugs were also fixed, and the build was deployed to Nexus > this time. But I am unsure whether the SCM is set correctly in pom.xml > - should it be tags/IMAGING_1_0_RC4 for a release? Also is > commons.rc.version correct? > > Tag: > > https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC4 > > Site: > http://people.apache.org/~damjan/imaging-1.0rc4/ > > Binaries: > > https://repository.apache.org/content/repositories/staging/org/apache/commons/commons-imaging/ > > Votes, please. This vote will close in 72 hours, Friday 28 September 2012 > at 04:33 GMT. > > [ ] +1 Release these artifacts > [ ] +0 OK, but... > [ ] -0 OK, but really should fix... > [ ] -1 I oppose this release because... > > Thank you! > > Damjan Jovanovic > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0 Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
