commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Damjan Jovanovic <dam...@apache.org>
Subject Re: [VOTE] Release Imaging 1.0 from RC7
Date Tue, 26 Nov 2013 14:50:55 GMT
On Tue, Nov 26, 2013 at 4:32 PM, Gary Gregory <garydgregory@gmail.com> wrote:
> On Tue, Nov 26, 2013 at 12:31 AM, Damjan Jovanovic <damjan@apache.org>wrote:
>
>> On Mon, Nov 25, 2013 at 1:50 PM, Emmanuel Bourg <ebourg@apache.org> wrote:
>> > Hi Damjan,
>>
>> Hi Emmanuel
>>
>> > I reviewed the API, here are my observations:
>>
>> Thank you.
>>
>> Firstly, we discussed several options before for the 1.0 release, and
>> agreed that the contents of trunk would be quickly pushed out as 1.0
>> with minimal changes (many/most users are using 1.0-SNAPSHOT), and
>> then the big API redesign would be 2.0. I've also been thinking of
>> doing a complete rewrite for 2.0 and only pulling in some of the good
>> bits we have now. So it's extremely discouraging to be pushed for more
>> and more changes, many of which will have no post-1.x value, and don't
>> even fit in with what was originally agreed on.
>>
>> > - Would that make sense to move the LZM implementation to
>> commons-compress?
>>
>> I think you mean LZW, and let's discuss that on COMPRESS-243.
>>
>> > - What is the purpose of CachingInputStream and CachingOutputStream,
>> > there is no Javadoc on these classes. Could they be merged into
>> commons-io?
>>
>> It looks like CachingInputStream is used by IccProfileParser, and
>> appears to be used to store data that has been read from the
>> underlying stream so it can be re-read later. You can copy it to
>> commons-io, but I'd prefer not having a runtime dependency on it. And
>> it's ByteSourceInputStream you really want in commons-io and/or
>> commons-compress, a gem that allows seeking over an InputStream.
>>
>> > - Why is ZLibUtils in org.apache.commons.imaging.common instead of
>> > org.apache.commons.imaging.util ?
>>
>> The whole org.apache.commons.imaging.util package is a bit dated.
>>
>> > - FastByteArrayOutputStream is only used by PackBits in the same
>> > package, it could be made package private.
>>
>> Yes.
>>
>> > - 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.
>>
>> Yes.
>>
>> > - There are several unused public methods in BinaryInputStream
>>
>> Yes.
>>
>> > - org.apache.commons.imaging.common.Compression is never used
>>
>> I imagine the idea was to use it instead of using the different
>> compression classes directly, but whoever started writing it never
>> finished.
>>
>> > - RgbBufferedImageFactory is only used in RoundtripTest, should this be
>> > moved with the test sources?
>>
>> Was probably meant to be the way to create all images in the API, but
>> again never got finished, and is probably wrong now since some image
>> formats produce paletted images.
>>
>> > - SimpleBufferedImageFactory is only used by ImageParser and could be
>> > made package private.
>>
>> And bears suspiciously high resemblance to RgbBufferedImageFactory.
>>
>> > - What about replacing org.apache.commons.imaging.common.ByteOrder with
>> > java.nio.ByteOrder?
>>
>> Enum vs public static final, hmm.
>>
>
> I think the point is that ByteOrder is a JRE provided class, we should not
> reinvent the wheel, unless we need to support middle-endian. Does Java run
> on PDP-11? ;)

You laugh, but the leDOS JVM runs on MS-DOS, and JAmiga on Amigas, so
it might be possible :).

Also by the same analogy, org.omg.CORBA had IntHolder since Java SE
1.2, yet we still have MutableInt in commons-lang. But either way,
Imaging's ByteOrder has already been replaced with java.nio's.

Damjan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message