You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2014/07/11 22:44:36 UTC
[VOTE] Release Commons CSV 1.0 based on RC1
We have worked a long time on CSV and more an more people seem to be using
the SNAPSHOT since there is no stable release. Although we still have some
outstanding issues regarding some corner cases and uncommon formats, It
feels like the API as reached a solid state. So I'd like to finally release
CSV 1.0.
CSV 1.0 RC1 is available for review here:
https://dist.apache.org/repos/dist/dev/commons/csv/ (svn rev. 5829)
(note that CSV can be build using Java 8.0, but the website cannot, since
the maven-findbugs-plugin is still uncompatible with Java 8.0)
Maven artifacts are here:
https://repository.apache.org/content/repositories/orgapachecommons-1039
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-RC1/changes-report.html
The tag is here:
http://svn.apache.org/repos/asf/commons/proper/csv/tags/CSV_1.0_RC1/ (svn
rev. 1609779)
N.B. the SVN revision is required because SVN tags are not immutable.
Site:
http://people.apache.org/~britter/csv-1.0-RC1/
(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-RC1/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 2300
GMT 14-July 2014
[ ] +1 Release these artifacts
[ ] +0 OK, but...
[ ] -0 OK, but really should fix...
[ ] -1 I oppose this release because...
Thanks!
Benedikt
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Benedikt Ritter <br...@apache.org>.
2014-07-12 3:17 GMT+02:00 Gary Gregory <ga...@gmail.com>:
> +1
>
> Apache Maven 3.2.2 (45f7c06d68e745d05611f7fd14efb6594181933e;
> 2014-06-17T09:51:42-04:00)
> Maven home: C:\Java\apache-maven-3.2.2
> Java version: 1.7.0_60, vendor: Oracle Corporation
> Java home: C:\Program Files\Java\jdk1.7.0_60\jre
> Default locale: en_US, platform encoding: Cp1252
> OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
>
> On Java 8, FindBugs fails, which is a know issue, which should be resolved
> when the 3.0.0 maven FindBugs plugin is released.
>
Note that FindBugs 3.0.0 requires Java 7. This may or may not be a problem
:-)
>
> Gary
>
>
> On Fri, Jul 11, 2014 at 4:44 PM, Benedikt Ritter <br...@apache.org>
> wrote:
>
> > We have worked a long time on CSV and more an more people seem to be
> using
> > the SNAPSHOT since there is no stable release. Although we still have
> some
> > outstanding issues regarding some corner cases and uncommon formats, It
> > feels like the API as reached a solid state. So I'd like to finally
> release
> > CSV 1.0.
> >
> > CSV 1.0 RC1 is available for review here:
> > https://dist.apache.org/repos/dist/dev/commons/csv/ (svn rev. 5829)
> > (note that CSV can be build using Java 8.0, but the website cannot,
> since
> > the maven-findbugs-plugin is still uncompatible with Java 8.0)
> >
> > Maven artifacts are here:
> >
> > https://repository.apache.org/content/repositories/orgapachecommons-1039
> >
> > 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-RC1/changes-report.html
> >
> > The tag is here:
> > http://svn.apache.org/repos/asf/commons/proper/csv/tags/CSV_1.0_RC1/
> > (svn
> > rev. 1609779)
> > N.B. the SVN revision is required because SVN tags are not immutable.
> >
> > Site:
> > http://people.apache.org/~britter/csv-1.0-RC1/
> > (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-RC1/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 2300
> > GMT 14-July 2014
> >
> > [ ] +1 Release these artifacts
> > [ ] +0 OK, but...
> > [ ] -0 OK, but really should fix...
> > [ ] -1 I oppose this release because...
> >
> > Thanks!
> >
> > Benedikt
> >
> > --
> > http://people.apache.org/~britter/
> > http://www.systemoutprintln.de/
> > http://twitter.com/BenediktRitter
> > http://github.com/britter
> >
>
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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
>
--
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
+1
Apache Maven 3.2.2 (45f7c06d68e745d05611f7fd14efb6594181933e;
2014-06-17T09:51:42-04:00)
Maven home: C:\Java\apache-maven-3.2.2
Java version: 1.7.0_60, vendor: Oracle Corporation
Java home: C:\Program Files\Java\jdk1.7.0_60\jre
Default locale: en_US, platform encoding: Cp1252
OS name: "windows 7", version: "6.1", arch: "amd64", family: "windows"
On Java 8, FindBugs fails, which is a know issue, which should be resolved
when the 3.0.0 maven FindBugs plugin is released.
Gary
On Fri, Jul 11, 2014 at 4:44 PM, Benedikt Ritter <br...@apache.org> wrote:
> We have worked a long time on CSV and more an more people seem to be using
> the SNAPSHOT since there is no stable release. Although we still have some
> outstanding issues regarding some corner cases and uncommon formats, It
> feels like the API as reached a solid state. So I'd like to finally release
> CSV 1.0.
>
> CSV 1.0 RC1 is available for review here:
> https://dist.apache.org/repos/dist/dev/commons/csv/ (svn rev. 5829)
> (note that CSV can be build using Java 8.0, but the website cannot, since
> the maven-findbugs-plugin is still uncompatible with Java 8.0)
>
> Maven artifacts are here:
>
> https://repository.apache.org/content/repositories/orgapachecommons-1039
>
> 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-RC1/changes-report.html
>
> The tag is here:
> http://svn.apache.org/repos/asf/commons/proper/csv/tags/CSV_1.0_RC1/
> (svn
> rev. 1609779)
> N.B. the SVN revision is required because SVN tags are not immutable.
>
> Site:
> http://people.apache.org/~britter/csv-1.0-RC1/
> (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-RC1/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 2300
> GMT 14-July 2014
>
> [ ] +1 Release these artifacts
> [ ] +0 OK, but...
> [ ] -0 OK, but really should fix...
> [ ] -1 I oppose this release because...
>
> Thanks!
>
> Benedikt
>
> --
> http://people.apache.org/~britter/
> http://www.systemoutprintln.de/
> http://twitter.com/BenediktRitter
> http://github.com/britter
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Jörg Schaible <jo...@gmx.de>.
Emmanuel Bourg wrote:
> Le 12/07/2014 17:57, Jörg Schaible a écrit :
>
>> According release notes Java 5 is supported, but it uses already methods
>> available in Java 6:
>
> It's even Java 7, CSVParser implements AutoCloseable.
I did no code analysis, but my Java 6 JDKs could compile and run the
complete build from the source tarball without any problem.
- Jörg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 12/07/2014 17:57, Jörg Schaible a écrit :
> According release notes Java 5 is supported, but it uses already methods
> available in Java 6:
It's even Java 7, CSVParser implements AutoCloseable.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Jörg Schaible <jo...@gmx.de>.
Hi Benedikt,
-1
According release notes Java 5 is supported, but it uses already methods
available in Java 6:
========================== %< ===========================
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-
plugin:3.1:compile (default-compile) on project commons-csv: Compilation
failure
[ERROR] ~/tmp/download/commons-csv-1.0-
src/src/main/java/org/apache/commons/csv/CSVParser.java:[384,77] cannot find
symbol
[ERROR] symbol : method isEmpty()
[ERROR] location: class java.lang.String
[ERROR] -> [Help 1]
========================== %< ===========================
Any newer JDK in my compiler zoo works fine though.
Cheers,
Jörg
Benedikt Ritter wrote:
> We have worked a long time on CSV and more an more people seem to be
> using
> the SNAPSHOT since there is no stable release. Although we still have some
> outstanding issues regarding some corner cases and uncommon formats, It
> feels like the API as reached a solid state. So I'd like to finally
> release CSV 1.0.
>
> CSV 1.0 RC1 is available for review here:
> https://dist.apache.org/repos/dist/dev/commons/csv/ (svn rev. 5829)
> (note that CSV can be build using Java 8.0, but the website cannot,
> since
> the maven-findbugs-plugin is still uncompatible with Java 8.0)
>
> Maven artifacts are here:
>
https://repository.apache.org/content/repositories/orgapachecommons-1039
>
> 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-RC1/changes-report.html
>
> The tag is here:
> http://svn.apache.org/repos/asf/commons/proper/csv/tags/CSV_1.0_RC1/
> (svn
> rev. 1609779)
> N.B. the SVN revision is required because SVN tags are not immutable.
>
> Site:
> http://people.apache.org/~britter/csv-1.0-RC1/
> (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-RC1/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 2300
> GMT 14-July 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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Jul 18, 2014 at 4:36 PM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 18/07/2014 22:17, Benedikt Ritter a écrit :
> > Just to make it clear... We're happy with:
> > - boolean isIgnoringSurroundingSpaces()
> > - withIgnoreSurroundingSpaces(boolean)
> >
> > So nothing has to change?
>
> Not sure, I find the asymmetry a bit disturbing, but if I'm the only one
> I can pass on that.
>
> I still feel better with the -ed form:
>
> withHeaderSkipped()
> isHeaderSkipped()
>
Yikes! That does not sounds good to me.
It is likely that we will not find a good set of names that sound good with
the 'with' prefix, as a getter, and as a standalone ivar name.
Gary
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 18/07/2014 22:17, Benedikt Ritter a écrit :
> Just to make it clear... We're happy with:
> - boolean isIgnoringSurroundingSpaces()
> - withIgnoreSurroundingSpaces(boolean)
>
> So nothing has to change?
Not sure, I find the asymmetry a bit disturbing, but if I'm the only one
I can pass on that.
I still feel better with the -ed form:
withHeaderSkipped()
isHeaderSkipped()
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Benedikt Ritter <be...@gmail.com>.
Just to make it clear... We're happy with:
- boolean isIgnoringSurroundingSpaces()
- withIgnoreSurroundingSpaces(boolean)
So nothing has to change?
Benedikt
Send from my mobile device
> Am 18.07.2014 um 16:12 schrieb Gary Gregory <ga...@gmail.com>:
>
> There is a nice pattern now with ivar and method names where we have the
> ivar ignoreSomething (note that it is verb first in the active void) and
> withIgnoreSomething.
>
> It is important IMO to use the active void instead of passive
> (somethingIgnored) to make it clear who performs the action.
>
> It's withIgnoreSurroundingSpaces, not withIgnoringSurroundingSpaces() BTW.
>
> So I do not think there is anything wrong with this one.
>
> Gary
>
>
>> On Fri, Jul 18, 2014 at 7:54 AM, Emmanuel Bourg <eb...@apache.org> wrote:
>>
>> Thank you for the fixes Benedikt.
>>
>> Le 15/07/2014 20:07, Benedikt Ritter a écrit :
>>
>>> skipHeaderRecord refers to the header records as a whole (so it's
>>> singular). ignore empty headers refers to header column values, so it's
>>> plural. I guess that makes sense.
>>
>> Ok, I misunderstood the intent then. Here "header" refers to a column
>> name, not the header record. What about renaming the property to
>> something like with/isUndefinedColumnIgnored() or
>> with/isEmptyColumnAllowed() to avoid the confusion?
>>
>>
>>> I've changed all methods to "is * ing". Now only isCommentingEnabled is
>>> left. I don't know what to do with this. I'm still looking forward to
>>> comments from a native speaker :)
>>
>> I tend to prefer withSurroundingSpacesIgnored over
>> withIgnoringSurroundingSpaces. Native speakers advices are welcome.
>>
>>
>>> I've tried to clarify the JavaDoc of the said methods. Can you please
>>> review?
>>
>> It looks good, thank you.
>>
>>
>>> No it doesn't. How about adding this in 1.1? I've created CSV-123 for
>> this.
>>
>> Ok, but we should document if the current method prints the header or not.
>>
>> Emmanuel Bourg
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>
>
> --
> E-Mail: garydgregory@gmail.com | ggregory@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
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
There is a nice pattern now with ivar and method names where we have the
ivar ignoreSomething (note that it is verb first in the active void) and
withIgnoreSomething.
It is important IMO to use the active void instead of passive
(somethingIgnored) to make it clear who performs the action.
It's withIgnoreSurroundingSpaces, not withIgnoringSurroundingSpaces() BTW.
So I do not think there is anything wrong with this one.
Gary
On Fri, Jul 18, 2014 at 7:54 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Thank you for the fixes Benedikt.
>
> Le 15/07/2014 20:07, Benedikt Ritter a écrit :
>
> > skipHeaderRecord refers to the header records as a whole (so it's
> > singular). ignore empty headers refers to header column values, so it's
> > plural. I guess that makes sense.
>
> Ok, I misunderstood the intent then. Here "header" refers to a column
> name, not the header record. What about renaming the property to
> something like with/isUndefinedColumnIgnored() or
> with/isEmptyColumnAllowed() to avoid the confusion?
>
>
> > I've changed all methods to "is * ing". Now only isCommentingEnabled is
> > left. I don't know what to do with this. I'm still looking forward to
> > comments from a native speaker :)
>
> I tend to prefer withSurroundingSpacesIgnored over
> withIgnoringSurroundingSpaces. Native speakers advices are welcome.
>
>
> > I've tried to clarify the JavaDoc of the said methods. Can you please
> > review?
>
> It looks good, thank you.
>
>
> > No it doesn't. How about adding this in 1.1? I've created CSV-123 for
> this.
>
> Ok, but we should document if the current method prints the header or not.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Jul 18, 2014 at 4:01 PM, Benedikt Ritter <be...@gmail.com>
wrote:
> I think "isAllowingMissingColumnNames" is a good name.
>
That does not real well at all unfortunately.
Gary
>
> Send from my mobile device
>
> > Am 18.07.2014 um 17:43 schrieb Emmanuel Bourg <eb...@apache.org>:
> >
> > Le 18/07/2014 17:37, Gary Gregory a écrit :
> >
> >> So what do you call the individual names in a header record?
> >
> > A column name. That's the term used in CSVRecord.
> >
> > Emmanuel Bourg
> >
> >
> > ---------------------------------------------------------------------
> > 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
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Benedikt Ritter <be...@gmail.com>.
I think "isAllowingMissingColumnNames" is a good name.
Send from my mobile device
> Am 18.07.2014 um 17:43 schrieb Emmanuel Bourg <eb...@apache.org>:
>
> Le 18/07/2014 17:37, Gary Gregory a écrit :
>
>> So what do you call the individual names in a header record?
>
> A column name. That's the term used in CSVRecord.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> 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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 18/07/2014 17:37, Gary Gregory a écrit :
> So what do you call the individual names in a header record?
A column name. That's the term used in CSVRecord.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Jul 18, 2014 at 11:35 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 18/07/2014 17:29, Gary Gregory a écrit :
>
> > There is indeed one header record. But that record contains one header
> per
> > column, so one could conceivably says that there are one or more headers
> > per record, just like there is one or more value per data record.
>
> That's what I found confusing. From my point of view a "header" is
> actually a header record, not a column name. Thus the header is defined
> as a set of column names.
>
So what do you call the individual names in a header record?
Gary
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 18/07/2014 17:29, Gary Gregory a écrit :
> There is indeed one header record. But that record contains one header per
> column, so one could conceivably says that there are one or more headers
> per record, just like there is one or more value per data record.
That's what I found confusing. From my point of view a "header" is
actually a header record, not a column name. Thus the header is defined
as a set of column names.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Jul 18, 2014 at 11:20 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 18/07/2014 16:13, Gary Gregory a écrit :
> > Would using "Headers" (plural) be better: withSkipHeadersRecord(boolean)
> ?
>
> Isn't the plural form withSkipHeaderRecords() instead?
>
> But since there is only one header record I'd keep the singular form.
>
I guess it depends what you are counting and how ;-)
There is indeed one header record. But that record contains one header per
column, so one could conceivably says that there are one or more headers
per record, just like there is one or more value per data record.
It's a subtle change but it sounds like it did not help since you did not
see it.
Gary
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 18/07/2014 16:13, Gary Gregory a écrit :
> Would using "Headers" (plural) be better: withSkipHeadersRecord(boolean) ?
Isn't the plural form withSkipHeaderRecords() instead?
But since there is only one header record I'd keep the singular form.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Gary Gregory <ga...@gmail.com>.
Would using "Headers" (plural) be better: withSkipHeadersRecord(boolean) ?
Gary
On Fri, Jul 18, 2014 at 7:54 AM, Emmanuel Bourg <eb...@apache.org> wrote:
> Thank you for the fixes Benedikt.
>
> Le 15/07/2014 20:07, Benedikt Ritter a écrit :
>
> > skipHeaderRecord refers to the header records as a whole (so it's
> > singular). ignore empty headers refers to header column values, so it's
> > plural. I guess that makes sense.
>
> Ok, I misunderstood the intent then. Here "header" refers to a column
> name, not the header record. What about renaming the property to
> something like with/isUndefinedColumnIgnored() or
> with/isEmptyColumnAllowed() to avoid the confusion?
>
>
> > I've changed all methods to "is * ing". Now only isCommentingEnabled is
> > left. I don't know what to do with this. I'm still looking forward to
> > comments from a native speaker :)
>
> I tend to prefer withSurroundingSpacesIgnored over
> withIgnoringSurroundingSpaces. Native speakers advices are welcome.
>
>
> > I've tried to clarify the JavaDoc of the said methods. Can you please
> > review?
>
> It looks good, thank you.
>
>
> > No it doesn't. How about adding this in 1.1? I've created CSV-123 for
> this.
>
> Ok, but we should document if the current method prints the header or not.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
--
E-Mail: garydgregory@gmail.com | ggregory@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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Thank you for the fixes Benedikt.
Le 15/07/2014 20:07, Benedikt Ritter a écrit :
> skipHeaderRecord refers to the header records as a whole (so it's
> singular). ignore empty headers refers to header column values, so it's
> plural. I guess that makes sense.
Ok, I misunderstood the intent then. Here "header" refers to a column
name, not the header record. What about renaming the property to
something like with/isUndefinedColumnIgnored() or
with/isEmptyColumnAllowed() to avoid the confusion?
> I've changed all methods to "is * ing". Now only isCommentingEnabled is
> left. I don't know what to do with this. I'm still looking forward to
> comments from a native speaker :)
I tend to prefer withSurroundingSpacesIgnored over
withIgnoringSurroundingSpaces. Native speakers advices are welcome.
> I've tried to clarify the JavaDoc of the said methods. Can you please
> review?
It looks good, thank you.
> No it doesn't. How about adding this in 1.1? I've created CSV-123 for this.
Ok, but we should document if the current method prints the header or not.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 18/07/2014 22:05, Benedikt Ritter a écrit :
> While looking at CSVFormat again, I noticed that the methods currently are withQuotePolicy and getQuotePolicy. Now I'm feeling we should go with the more verbose but more explicit name for that enum.
Ok. We could use QuoteMode instead of QuotePolicy, that's slightly
shorter and similar to the *Mode enums in the Hibernate API.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Benedikt Ritter <be...@gmail.com>.
Send from my mobile device
> Am 15.07.2014 um 20:07 schrieb Benedikt Ritter <br...@apache.org>:
>
> Hello Emmanuel,
>
> thanks for your thorough review. I've worked through your list. Here are my comments:
>
>
> 2014-07-12 19:02 GMT+02:00 Emmanuel Bourg <eb...@apache.org>:
>> I took a fresh look at the API and here is my review based on the
>> Javadoc and the content of the site only:
>>
>> - The Javadoc for CSVFormat mentions a parseFile() method, but this
>> method doesn't exist.
>
> fixed
>
>>
>> - CSVFormat.DEFAULT states that empty lines are "allowed". Does it mean
>> the empty lines are ignored or returned as an empty record? EDIT:
>> Actually it's properly documented but I got confused by the style of the
>> <h3> section. It's rendered like a field header and I thought that was
>> the documentation of the next field.
>
> fixed
>
>>
>> - CSVFormat.DEFAULT, EXCEL and RFC4180 mention
>> 'withRecordSeparator(CRLF)', but CRLF is not visible in the Javadoc.
>
> fixed
>
>>
>> - CSVFormat.newFormat() doesn't tell the value of the other settings. I
>> assume it's based on the DEFAULT format, but that's not obvious.
>
> fixed
>
>>
>> - There is with/getIgnoreEmptyHeaders() but unlike lines there is only
>> one header. So we should probably remove the 's'. That would be
>> consistent with skipHeaderRecord (no 's').
>
> skipHeaderRecord refers to the header records as a whole (so it's singular). ignore empty headers refers to header column values, so it's plural. I guess that makes sense.
>
>>
>> - I see an inconsistency with the CSVFormat.is*() methods. There is a
>> mix of is*ingEnabled() and is*ing(). If a native English speaker could
>> look at these methods and tell if they are ok that would be great. For
>> example I wonder if isNullHandling() should be turned into
>> isHandlingNull() or isNullHandled().
>
> I've changed all methods to "is * ing". Now only isCommentingEnabled is left. I don't know what to do with this. I'm still looking forward to comments from a native speaker :)
>
>>
>> - I'm not fond of the CSVFormat.get*() methods returning a boolean. For
>> example I'd prefer isHeaderRecordSkipped() instead of getSkipHeaderRecord().
>
> fixed
>
>>
>> - CSVFormat.withRecordSeparator() could state explicitly it doesn't
>> affect the parsing (i.e., if configured with \r\n, an input file with \n
>> only will be properly parsed).
>
> fixed
>
>>
>> - That may sound dumb but I see CSVFormat.withCommentStart() and
>> intuitively expect a withCommentStop() method, probably because my
>> programmer's mind thinks about a block comment of a programming
>> language, but that doesn't exist in the CSV format. Maybe
>> withCommentMarker() would be less confusing.
>
> fixed
>
>>
>> - looking at the Javadoc of CSVPrinter I don't understand how
>> printRecords() works and how it differs from printRecord() (same
>> signatures).
>
> I've tried to clarify the JavaDoc of the said methods. Can you please review?
>
>>
>> - Does CSVPrinter.printRecord(ResultSet) print a header line derived
>> from the metadata? I would add a boolean parameter to control this.
>
> No it doesn't. How about adding this in 1.1? I've created CSV-123 for this.
>
>>
>> - CSVPrinter has a println() method, this sparks a doubt: do I have to
>> call this after each printRecord()? An example in the class
>> documentation could clear that.
>
> I've added a hint in the JavaDoc of printRecord.
>
>>
>> - The documentation for CSVRecord.isConsistent() could be improved. I
>> would rewrite the first sentence as "Tells if the record size matches
>> the header size". This is what will be displayed in the method table and
>> better conveys the purpose of this method than "Returns true if this
>> record is consistent", which is somewhat obvious for isConsistent() :)
>
> fixed
>
>>
>> - The documentation for CSVRecord.toString() could specify the output
>> format.
>
> Currently we're just calling Arrays.toString with the values, which could be improved. I've created CSV-124 for this.
>
>>
>> - The documentation of CSVRecord.getRecordNumber() may elaborate on the
>> difference between line number and record number. A reference to
>> CSVParser.getCurrentLineNumber() would be useful.
>
> fixed
>
>>
>> - CSVParser.getRecords(T records) doesn't look useful to me, I'd remove
>> it and suggest using records.addAll(parser.getRecords()) instead.
>
> Agreed, I've removed the method.
>
>>
>> - Quote vs QuotePolicy: "Quote" is shorter but "QuotePolicy" is more
>> explicit, I'm not sure which one I prefer.
>
> I neither, so I leave it as it is :o)
While looking at CSVFormat again, I noticed that the methods currently are withQuotePolicy and getQuotePolicy. Now I'm feeling we should go with the more verbose but more explicit name for that enum.
>
>> - The <p> elements after <h2> create an excessive space in the Javadoc,
>> I suggest removing them.
>
> Removing the <p> does only remove the space *after* the tag. The excessive space before is the result of the <h2> tags.
>
>>
>> - The code examples in the userguide use a two spaces indentation :)
>
> fixed
>
>>
>> - The documentation of CSVFormat could be copied in the userguide to
>> better show how to manipulate the API.
>
> I don't like having the same content in two places. I've added an additional link to the website. Anyway, the website can easily bu updated after the release ;-)
>
>>
>>
>> That looks like a lot of comments by I'm really pleased with the API,
>> congratulations everyone I'm glad we are near a release.
>
> Me too! I'll create RC2 now.
>
>>
>> Emmanuel Bourg
>>
>>
>> ---------------------------------------------------------------------
>> 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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Benedikt Ritter <br...@apache.org>.
Hello Emmanuel,
thanks for your thorough review. I've worked through your list. Here are my
comments:
2014-07-12 19:02 GMT+02:00 Emmanuel Bourg <eb...@apache.org>:
> I took a fresh look at the API and here is my review based on the
> Javadoc and the content of the site only:
>
> - The Javadoc for CSVFormat mentions a parseFile() method, but this
> method doesn't exist.
>
fixed
>
> - CSVFormat.DEFAULT states that empty lines are "allowed". Does it mean
> the empty lines are ignored or returned as an empty record? EDIT:
> Actually it's properly documented but I got confused by the style of the
> <h3> section. It's rendered like a field header and I thought that was
> the documentation of the next field.
>
fixed
>
> - CSVFormat.DEFAULT, EXCEL and RFC4180 mention
> 'withRecordSeparator(CRLF)', but CRLF is not visible in the Javadoc.
>
fixed
>
> - CSVFormat.newFormat() doesn't tell the value of the other settings. I
> assume it's based on the DEFAULT format, but that's not obvious.
>
fixed
>
> - There is with/getIgnoreEmptyHeaders() but unlike lines there is only
> one header. So we should probably remove the 's'. That would be
> consistent with skipHeaderRecord (no 's').
>
skipHeaderRecord refers to the header records as a whole (so it's
singular). ignore empty headers refers to header column values, so it's
plural. I guess that makes sense.
>
> - I see an inconsistency with the CSVFormat.is*() methods. There is a
> mix of is*ingEnabled() and is*ing(). If a native English speaker could
> look at these methods and tell if they are ok that would be great. For
> example I wonder if isNullHandling() should be turned into
> isHandlingNull() or isNullHandled().
>
I've changed all methods to "is * ing". Now only isCommentingEnabled is
left. I don't know what to do with this. I'm still looking forward to
comments from a native speaker :)
>
> - I'm not fond of the CSVFormat.get*() methods returning a boolean. For
> example I'd prefer isHeaderRecordSkipped() instead of
> getSkipHeaderRecord().
>
fixed
>
> - CSVFormat.withRecordSeparator() could state explicitly it doesn't
> affect the parsing (i.e., if configured with \r\n, an input file with \n
> only will be properly parsed).
>
fixed
>
> - That may sound dumb but I see CSVFormat.withCommentStart() and
> intuitively expect a withCommentStop() method, probably because my
> programmer's mind thinks about a block comment of a programming
> language, but that doesn't exist in the CSV format. Maybe
> withCommentMarker() would be less confusing.
>
fixed
>
> - looking at the Javadoc of CSVPrinter I don't understand how
> printRecords() works and how it differs from printRecord() (same
> signatures).
>
I've tried to clarify the JavaDoc of the said methods. Can you please
review?
>
> - Does CSVPrinter.printRecord(ResultSet) print a header line derived
> from the metadata? I would add a boolean parameter to control this.
>
No it doesn't. How about adding this in 1.1? I've created CSV-123 for this.
>
> - CSVPrinter has a println() method, this sparks a doubt: do I have to
> call this after each printRecord()? An example in the class
> documentation could clear that.
>
I've added a hint in the JavaDoc of printRecord.
>
> - The documentation for CSVRecord.isConsistent() could be improved. I
> would rewrite the first sentence as "Tells if the record size matches
> the header size". This is what will be displayed in the method table and
> better conveys the purpose of this method than "Returns true if this
> record is consistent", which is somewhat obvious for isConsistent() :)
>
fixed
>
> - The documentation for CSVRecord.toString() could specify the output
> format.
>
Currently we're just calling Arrays.toString with the values, which could
be improved. I've created CSV-124 for this.
>
> - The documentation of CSVRecord.getRecordNumber() may elaborate on the
> difference between line number and record number. A reference to
> CSVParser.getCurrentLineNumber() would be useful.
>
fixed
>
> - CSVParser.getRecords(T records) doesn't look useful to me, I'd remove
> it and suggest using records.addAll(parser.getRecords()) instead.
>
Agreed, I've removed the method.
>
> - Quote vs QuotePolicy: "Quote" is shorter but "QuotePolicy" is more
> explicit, I'm not sure which one I prefer.
>
>
I neither, so I leave it as it is :o)
> - The <p> elements after <h2> create an excessive space in the Javadoc,
> I suggest removing them.
>
Removing the <p> does only remove the space *after* the tag. The excessive
space before is the result of the <h2> tags.
>
> - The code examples in the userguide use a two spaces indentation :)
>
fixed
>
> - The documentation of CSVFormat could be copied in the userguide to
> better show how to manipulate the API.
>
I don't like having the same content in two places. I've added an
additional link to the website. Anyway, the website can easily bu updated
after the release ;-)
>
>
> That looks like a lot of comments by I'm really pleased with the API,
> congratulations everyone I'm glad we are near a release.
>
Me too! I'll create RC2 now.
>
> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> 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
Re: [VOTE] Release Commons CSV 1.0 based on RC1
Posted by Emmanuel Bourg <eb...@apache.org>.
I took a fresh look at the API and here is my review based on the
Javadoc and the content of the site only:
- The Javadoc for CSVFormat mentions a parseFile() method, but this
method doesn't exist.
- CSVFormat.DEFAULT states that empty lines are "allowed". Does it mean
the empty lines are ignored or returned as an empty record? EDIT:
Actually it's properly documented but I got confused by the style of the
<h3> section. It's rendered like a field header and I thought that was
the documentation of the next field.
- CSVFormat.DEFAULT, EXCEL and RFC4180 mention
'withRecordSeparator(CRLF)', but CRLF is not visible in the Javadoc.
- CSVFormat.newFormat() doesn't tell the value of the other settings. I
assume it's based on the DEFAULT format, but that's not obvious.
- There is with/getIgnoreEmptyHeaders() but unlike lines there is only
one header. So we should probably remove the 's'. That would be
consistent with skipHeaderRecord (no 's').
- I see an inconsistency with the CSVFormat.is*() methods. There is a
mix of is*ingEnabled() and is*ing(). If a native English speaker could
look at these methods and tell if they are ok that would be great. For
example I wonder if isNullHandling() should be turned into
isHandlingNull() or isNullHandled().
- I'm not fond of the CSVFormat.get*() methods returning a boolean. For
example I'd prefer isHeaderRecordSkipped() instead of getSkipHeaderRecord().
- CSVFormat.withRecordSeparator() could state explicitly it doesn't
affect the parsing (i.e., if configured with \r\n, an input file with \n
only will be properly parsed).
- That may sound dumb but I see CSVFormat.withCommentStart() and
intuitively expect a withCommentStop() method, probably because my
programmer's mind thinks about a block comment of a programming
language, but that doesn't exist in the CSV format. Maybe
withCommentMarker() would be less confusing.
- looking at the Javadoc of CSVPrinter I don't understand how
printRecords() works and how it differs from printRecord() (same
signatures).
- Does CSVPrinter.printRecord(ResultSet) print a header line derived
from the metadata? I would add a boolean parameter to control this.
- CSVPrinter has a println() method, this sparks a doubt: do I have to
call this after each printRecord()? An example in the class
documentation could clear that.
- The documentation for CSVRecord.isConsistent() could be improved. I
would rewrite the first sentence as "Tells if the record size matches
the header size". This is what will be displayed in the method table and
better conveys the purpose of this method than "Returns true if this
record is consistent", which is somewhat obvious for isConsistent() :)
- The documentation for CSVRecord.toString() could specify the output
format.
- The documentation of CSVRecord.getRecordNumber() may elaborate on the
difference between line number and record number. A reference to
CSVParser.getCurrentLineNumber() would be useful.
- CSVParser.getRecords(T records) doesn't look useful to me, I'd remove
it and suggest using records.addAll(parser.getRecords()) instead.
- Quote vs QuotePolicy: "Quote" is shorter but "QuotePolicy" is more
explicit, I'm not sure which one I prefer.
- The <p> elements after <h2> create an excessive space in the Javadoc,
I suggest removing them.
- The code examples in the userguide use a two spaces indentation :)
- The documentation of CSVFormat could be copied in the userguide to
better show how to manipulate the API.
That looks like a lot of comments by I'm really pleased with the API,
congratulations everyone I'm glad we are near a release.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org