commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [VOTE] Release Commons CSV 1.0 based on RC3
Date Mon, 11 Aug 2014 05:06:32 GMT
Hello Bernd,

thanks for the review. See my comments inline.


2014-08-10 18:58 GMT+02:00 Bernd Eckenfels <ecki@zusammenkunft.net>:

> Hello,
>
> +0 (non-binding if you like :)
> the sandbox should really be removed (see below)
>
> checked out tag (rev matches)
>
> - build with 3.2.1/7u65/Win7: install and site ok
> - build with 3.0.5/6u38/Win7: install ok
>   (site fails with findbugs class version problem)
>

Yes, the new findbugs version only works with Java 7.0+


> - build with 3.2.1/8u11/Win7: install and site ok
>   (javadoc warnings)
>
>
> site/mail-lists talk about sandbox
>

One thing to remember is, that we're only releasing source code. So issues
with the website should never delay a release IMHO (others have expressed
that say see this differently...)


>
> 2 CSVParser tests skipped, is this expected?
>

Yes, it is expected. Although I agree that igorned tests are bad practice...


>
>
> Plugins are up to date, the maven min version could be adjusted
> (findbugs needs 3.0.1).
>

I'll update that after the release.


>
> Dependencies are up-to-date (only a newer JUnit beta).
>
> Nit: the changes report show CVS-125 first, thats confusing as I
> understand it does work with Java6 - partially?
>

The issues are ordered the way they are documented in changes.xml. The
issue was raised, when the min Java version was changed to Java 7.0.


>
> What about the PMD warning with the unused print() method?
>

The method is actually used a few lines above its declaration. I don't know
why it shows up in the PMD report. I've seen this behavior in lang as
well...


>
> Do we want to create a wiki page so the link works?
> http://wiki.apache.org/commons/CSV
>
>
Yes why not.


>
>
> The following stuff i might report/contribute and are not strictly 1.0
> problems:
>
> Nit: it should not talk about "Excel format" but something like "CSV
> files exported by Microsoft Excel". We should honor the trademark here.
>

Good point, maybe Phil can comment here, since he knows more about the
legal trademark stuff (at least that's the impression he has made upon me
:-)


>
> Nit: the package-doc+user guide+CSVPrinter javadoc should describe this
> mode of function some more.
>

Okay, another thing for the 1.1 TODO list :o)


>
>
> Overall very good quality!
>

Thanks!
Benedikt


>
> Gruss
> Bernd
>
>
>
>
>
>
> Am Sun, 10 Aug 2014 14:37:47 +0200
> schrieb Benedikt Ritter <britter@apache.org>:
>
> > Hi all,
> >
> > we have further refined the API and I'd like to finally release
> > Commons CSV based on RC3.
> >
> > Changes between RC2 and RC3 are:
> >
> > - reverted to Java 6 as language level requirement
> > - updated dependencies to the latest versions
> > - refined the API of CSVFormat some more
> >
> > Changes between RC1 und RC2 where:
> >
> > - CSV now requires Java 7 and the Java requirements are documented in
> > the release notes and on the website.
> > - corrected JavaDoc in CSVFormat that was pointing to missing method
> > parseFile (the JavaDoc now correctly points to the static methods
> > defined in CSVParser)
> > - The JavaDoc for the predefined formats in CSVFormat doesn't use h3
> > sections any more, since this is not good for readbility
> > - The JavaDoc for the predefined formats in CSVFormat now uses the
> > actual characters and not the (not visibile) constants from the
> > Constants class
> > - getters returning booleans in CSVFormat now follow the convention of
> > starting with "is" instead of "get"
> > - consistently renamed boolean getters
> > - Clarified JavaDoc of CSVFormat.withRecordSeparator()
> > - Renamed CSVFormat.withCommentStart and getCommentStart to
> > with/getCommentMarker to avoid confusion w.r.t. comment stop
> > - Clarified JavaDoc of CSVPrinter.printRecord(s) methods
> > - Clarified in JavaDoc, that CSVPrinter.printRecord also calls
> > CSVPrinter.println()
> > - Clarified toString format of CSVPrinter
> > - Clarified the difference between the current line number of the
> > parser and the record number of the record
> > - Removed needless method getRecord(T extends Collection)
> > - use 4 spaces indentation in user guide
> >
> > CSV 1.0 RC3 is available for review here:
> >     https://dist.apache.org/repos/dist/dev/commons/csv/ (svn rev.
> > 6117)
> >
> > I've tested with Maven 3.2.2 using Java 6, 7 and 8.
> >
> >   Maven artifacts are here:
> >
> https://repository.apache.org/content/repositories/orgapachecommons-1043/
> >
> >   Details of changes since we started with 1.0 are in the release
> > notes:
> > https://dist.apache.org/repos/dist/dev/commons/csv/RELEASE-NOTES.txt
> > http://people.apache.org/~britter/csv-1.0-RC3/changes-report.html
> >
> >   The tag is here:
> >     https://svn.apache.org/repos/asf/commons/proper/csv/tags/CSV_1.0_RC3
> > (svn rev. 1617100)
> >   N.B. the SVN revision is required because SVN tags are not
> > immutable.
> >
> >   Site:
> >     http://people.apache.org/~britter/csv-1.0-RC3/
> >   (note some *relative* links are broken and the 1.0 directories are
> >   not yet created - these will be OK once the site is deployed)
> >
> >   Clirr Report:
> >    -- No Clirr report, since this is the first release --
> >
> >   RAT Report:
> >     http://people.apache.org/~britter/csv-1.0-RC3/rat-report.html
> >   (note that the files in src/test/resources are ignored by rat,
> > since they are used as test csv input   and test result
> > specifications)
> >
> >   KEYS:
> >   http://www.apache.org/dist/commons/KEYS
> >
> >   Please review the release candidate and vote.
> >   This vote will close no sooner that 72 hours from now, i.e. after
> > 1500 GMT 13-August 2014
> >
> >   [ ] +1 Release these artifacts
> >   [ ] +0 OK, but...
> >   [ ] -0 OK, but really should fix...
> >   [ ] -1 I oppose this release because...
> >
> >   Thanks!
> > Benedikt
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message