commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "dam6923 ." <dam6...@gmail.com>
Subject Re: [CANCELLED][VOTE] Release Commons Compress 1.6
Date Mon, 14 Oct 2013 16:03:02 GMT
Just a couple of immediate thoughts (just starting to look things over...)

In SevenZFile.java

Constructor...
1) Close file on exception instead of the current technique of keeping
a "succeeded" flag.
2) Move setting the password to the last line of the constructor so
that it isn't stored if an exception is thrown at any point.

General...
1) I see a few instances of
        if (file != null) {
            try {
                file.close();
            } catch (IOException ignored) { // NOPMD
            }
            file = null;
        }
Can we snag the the IoUtils close methods in this project?

2) The class JavaDoc comments are wrapped after about 50 characters..
extend to 80?
3) There's a 'leaked resource' warning on line 190.  A
ByteArrayInputStream is not being closed, which makes sense as closing
it does nothing, but for completeness sake perhaps or to protect
against a time that the input-source could change and then it really
is leaking.
4) Non-standard logging.  There are many logging statements in the
code... perhaps delegate that to a known logging framework or remove
it all entirely?
5) Line 870 - magic number

Thanks,
David

On Mon, Oct 14, 2013 at 12:17 AM, Stefan Bodewig <bodewig@apache.org> wrote:
> I think enough issues have been identified to warrant a second RC.
>
> I'll take care of the bad version number in the release notes as well as
> the PMD warnings in the ARJ-Package.  ArjArchiveEntry's test coverage
> has already been increased in trunk.
>
> We should talk about the Clirr report further and come to a conclusion
> what to do about the changes - if anything at all.
>
> For the site's contents I don't see myself working on it before a second
> RC.  Help is more than welcome (not only with the site, of course).
>
> Thanks to all who took the time to review the RC
>
>        Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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


Mime
View raw message