Hello Bernd, thanks for the review. See my comments inline.
2014-08-10 18:58 GMT+02:00 Bernd Eckenfels <e...@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 <brit...@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-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > -- http://people.apache.org/~britter/ http://www.systemoutprintln.de/ http://twitter.com/BenediktRitter http://github.com/britter