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-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org >
signature.asc
Description: OpenPGP digital signature
