On Fri, Nov 1, 2013 at 8:24 PM, Gary Gregory <garydgreg...@gmail.com> wrote:

> Thank you Damjan for cutting another release!
>
> In the future, please summarize changes from RC to RC in the VOTE email.
> The link to the VOTE thread is too much of a hunt to understand what's
> changed. I might as well look at SVN.
>
> I admire Damjan's persistence and patience in seeing through another RC
> toward 1.0 :)
>

Thank you :).


> [X] -0 OK, but really should fix...
>
> - The RAT report shows "40 Unknown Licenses", either add license headers or
> get RAT to ignore these files.
>

RAT's Maven plugin doesn't have a setting to ignore files. I've merged the
info.txt files that state where each test image came from into one file and
put a license header in it. XPM/XBM/PAM/PGM are text-based images, so it
may not be possible to comment them.
src/main/resources/org/apache/commons/imaging/formats/xpm/rgb.txt is not
our file - it's an MIT licensed file from the X.org project as stated in
LICENSE.txt, so should we put an MIT header in it?


> - On the "References" page, all the links to Geocities are broken. Either
> remove the broken links or link to the internet archive wayback machine or
> similar.
>

Done.


> - Is the to-do list on the site 100% accurate?
> https://people.apache.org/~damjan/imaging-1.0-RC5/todo.html
>

Fixed it up.


> - I would also change the title of the page from "To Do" to "Roadmap" and
> talk about the upcoming 2.0 plan how 1.0 relates to 0.97 (new package).
>

It's "Roadmap" now.


> - We need a "Migrating from 0.97 to 1.0" and "Plans for 2.0" sections, IMO.
>

Judging by the bug reports, almost everyone is using 1.0-SNAPSHOT. Version
0.97 is a pre-release incubator version, and there's already documentation
about the transition in RELEASE-NOTES.txt and in the "History" page.


> - For the page "Project Status and History", I would just call it
> "History". This page is missing versions.
>

I did. Added the 2 versions that were missing.


> - Javadoc: Most packages are missing a package-level description.
>

Added it for the main packages.


> - Findbugs reports
> org.apache.commons.imaging.icc.IccProfileParser.issRGB(ICC_Profile) has
> Boolean return type and returns explicit null BAD_PRACTICE (and others like
> it).
> For methods like
> org.apache.commons.imaging.icc.IccProfileParser.issRGB(ICC_Profile), it
> seems to complicate the code a lot more to return a Boolean instead of a
> boolean.
> We should consider changing these APIs to boolean. I've only looks at
> IccProfileParser.issRGB
> and this one case seems OK for change.
>

The problem was that issRGB had failure states, such as when the input is
null, or when an exception gets thrown (it swallows them all), for which it
returns null. But I've made it return boolean and throw exceptions instead.


> - Checkstyle: We have a lot of 'Useless parentheses', the few I've checked
> seem like they should be removed. This is a lot of work to check one at a
> time and I do not think we should remove them all automatically (like
> Eclipse can do) unless the person most familiar with the code chooses to do
> so.
>

I am not sure which parentheses you mean, the () around expressions, or the
{} around statements. Which Checkstyle settings are you using?


> - I'm not a fan of using interfaces (instead of classes) to define
> constants, but if this in fact the style we're going forward with, we
> should not redundantly define each constant as public, a fact that is
> implied by being an interface member (and which Checkstyle complains
> about.)
>

The problem is that there are endless TIFF tags, so they're convenient to
split up into separate classes when implementing, but then difficult for
the user to find the right one. So they've been merged into AllTagConstants
by multi-inheritance, something classes cannot do. But I've deleted public
static final at least, and converted XyzConstants to classes in other
packages.


> - Findbugs: WRT byte arrays and "malicious code", it seems safe to ignore
> these since we are working with bytes all the time. I'm not sure if it is
> worth Javadoc'ing this intention or adding some kinds of comment in the
> source Findbugs can use to avoid these warnings. It's probably NOT worth
> doing, but I thought I'd mention it.
>

There is no source comment. Findbugs can use a filter to not examine
certain files/classes/methods, but I am not a fan - that filter could hide
other bugs in the future.


> Tested with:
>
> Apache Maven 3.1.1 (0728685237757ffbf44136acec0402957f723d9a; 2013-09-17
> 11:22:22-0400)
> Maven home: C:\Java\apache-maven-3.1.1\bin\..
> Java version: 1.7.0_45, vendor: Oracle Corporation
> Java home: C:\Program Files\Java\jdk1.7.0_45\jre
> Default locale: en_US, platform encoding: Cp1252
> OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
>
> Gary
>
>
Damjan


>
> On Fri, Nov 1, 2013 at 7:57 AM, Damjan Jovanovic <dam...@apache.org>
> wrote:
>
> > Please vote on releasing commons-imaging 1.0 from RC5.
> >
> > RC4 and its problems and their fixes were in this thread:
> >
> >
> http://mail-archives.apache.org/mod_mbox/commons-dev/201209.mbox/%3CCAJm2B-nbnbJwNUKkAtapZuzT5jfFODsk1aXcdsUUeoC%2BxXrDKg%40mail.gmail.com%3E
> > There were also many recent discussions on the development list.
> >
> > Imaging 1.0 RC5 is available here:
> > https://dist.apache.org/repos/dist/dev/commons/imaging/ (SVN revision
> > 3391)
> >
> > 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-RC5/changes-report.html
> > http://people.apache.org/~damjan/imaging-1.0-RC5/jira-report.html
> >
> > Tag:
> >
> >
> https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC5
> > <
> >
> https://svn.apache.org/repos/asf/commons/proper/imaging/tags/IMAGING_1_0_RC4
> > >(SVN
> > revision 1537825)
> >
> > Site:
> > http://people.apache.org/~damjan/imaging-1.0-RC5/<
> > http://people.apache.org/%7Edamjan/imaging-1.0rc4/>
> >
> > 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 Monday 4 November 2013 at 12: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
> >
>
>
>
> --
> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> Java Persistence with Hibernate, Second Edition<
> http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>

Reply via email to