commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Bourg <ebo...@apache.org>
Subject Re: [VOTE] Release Imaging 1.0 from RC7
Date Mon, 25 Nov 2013 11:50:34 GMT
Hi Damjan,

I reviewed the API, here are my observations:

- Would that make sense to move the LZM implementation to commons-compress?

- What is the purpose of CachingInputStream and CachingOutputStream,
there is no Javadoc on these classes. Could they be merged into commons-io?

- Why is ZLibUtils in org.apache.commons.imaging.common instead of
org.apache.commons.imaging.util ?

- FastByteArrayOutputStream is only used by PackBits in the same
package, it could be made package private.

- BitArrayOutputStream in org.apache.commons.imaging.common is only used
by classes in org.apache.commons.imaging.common.itu_t4. It could be
moved into this package and made package private.

- There are several unused public methods in BinaryInputStream

- org.apache.commons.imaging.common.Compression is never used

- RgbBufferedImageFactory is only used in RoundtripTest, should this be
moved with the test sources?

- SimpleBufferedImageFactory is only used by ImageParser and could be
made package private.

- What about replacing org.apache.commons.imaging.common.ByteOrder with
java.nio.ByteOrder?

- Several methods in BinaryFunctions have an unused 'name' parameter.

- UnicodeUtils is only used by PngWriter and could be made package
private. Considering that any byte sequence is a valid ISO-8859-1 string
this class could also be removed.

- Why does RationalNumberUtilities extend Number? And shouldn't this
class be named RationalNumberUtils for consistency with the other *Utils
classes? I think the class could simply be merged into RationalNumber.

- There are several unused methods in ColorTools, I don't know if they
are actually useful and are just missing a unit test.

- ZigZag is only used in JpegDecoder and could be made package private

- The public permissive field in JpegImageParser is never used

- The public Attribute_Tag field in TiffField is never used

- Some public static final constants don't have an uppercased name, I
fixed them on the trunk

- The PixelParser classes in
org.apache.commons.imaging.formats.bmp.pixelparsers seems to be used
only internally by BmpImageParser. They can't be made package private
but could at least be excluded from the Javadoc.

- Same comment for org.apache.commons.imaging.formats.bmp.writers

- The Dct, JpegInputStream and YCbCrConverter classes in
org.apache.commons.imaging.formats.jpeg.decoder are only used internally
by JpegDecoder and could be made package private.

- What about moving the methods in ColorConversions into the respective
Color classes? For example, ColorConversions.convertCIELuvtoXYZ() could
become ColorCieLuv.toXYZ(). Another idea would be to use a fluent
interface to perform the conversions. Something like
ColorConverter.fromRGB(r, g, b).toHSL().


I haven't reviewed everything yet, but I have the feeling the public API
could be better polished to remove unused or internal stuff that are
probably not useful for the end users.

Emmanuel Bourg



Le 24/11/2013 17:54, Damjan Jovanovic a écrit :
> Please vote on releasing commons-imaging 1.0 from RC7.
> 
> Last failed vote was for RC5, RC6 had a bust POM. Many improvements
> were made since:
> * PMD configured better and all PMD warnings fixed, the ones remaining
> now are bugs in PMD itself.
> * Test coverage greatly improved in many areas, only a few packages
> now have < 50% coverage, and that's due to obscure TIFF photometric
> interpreters and PSD data parsers that require special images to test
> which Imaging can't itself generate, a massive barely used
> semi-obsolete Debug class that drags down coverage for
> org.apache.commons.imaging.util, and areas of code I don't understand
> and so can't easily test (ICC, PSD). While improving test coverage, I
> also found and fixed 2 bugs.
> * Added package-level Javadoc for image formats.
> * RAT exclusions for text-based images, and made a single info.txt
> with image formation for all images and added a license header to it.
> * Fixed Jacoco configuration in the POM and started using it instead
> of that 40+ minute Cobertura nonsense.
> * Fixed website broken links.
> * Also started assembling a list of why making releases is such a
> pain, email coming later :).
> 
> Imaging 1.0 RC7 is available here:
> https://dist.apache.org/repos/dist/dev/commons/imaging/ (SVN revision 3671)
> 
> Maven artifacts:
> https://repository.apache.org/content/repositories/staging/org/apache/commons/commons-imaging/
> 
> Change log(s):
> https://dist.apache.org/repos/dist/dev/commons/imaging/RELEASE-NOTES.txt
> http://people.apache.org/~damjan/imaging-1.0-RC7/changes-report.html
> http://people.apache.org/~damjan/imaging-1.0-RC7/jira-report.html
> 
> Tag:
> https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC7
> (SVN revision 1544953)
> 
> Site:
> http://people.apache.org/~damjan/imaging-1.0-RC7/
> 
> KEYS:
> http://www.apache.org/dist/commons/KEYS
> 
> I have tested it with OpenJDK 6 on FreeBSD 9.1.
> 
> Please review and vote. This vote will close no sooner than 72 hours
> from now, on Wednesday 27 November 2013 at 19:00 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
> 



Mime
View raw message