commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [VOTE] Release Imaging 1.0 from RC4
Date Tue, 25 Sep 2012 13:22:23 GMT
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.jov@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message