Ok, I don't see any conflict anymore but the PR title does not match the code: "CSVFormat.duplicateHeaderMode requires default DISALLOW"
I am ok with the changes as they stand now but we have to resolve if the default should be changed. Gary On Fri, Oct 21, 2022, 09:44 <sma...@outlook.de> wrote: > Hi Gary, > > the PR did not have conflicts 30 minutes ago. So we must have encountered > a race condition between you updating master and me rebasing and > force-pushing my pr. > I've done that again just now. There are no conflicts. > At the same time I see parts of my PR have dribbled into master already, > so if you rather implement the changes yourself, please go ahead. I am fine > with closing my pr. > > regards, > Markus > > > From: Gary Gregory <garydgreg...@gmail.com> > Sent: Friday, October 21, 2022 15:17 > To: Commons Developers List <dev@commons.apache.org> > Subject: Re: [VOTE] Release Apache Commons CSV 1.10.0 based on RC1 > > Hi Markus, > > The PR has conflicts and does not test its changes. > > I'd like feedback from the community on whether or not git master as it is > now is OK for the duplicate header behavior or if changing the default is > needed and if doing so is just ping-pongonging from one incompatibility to > another, based on previous versions. > > TY! > > Gary > > On Fri, Oct 21, 2022, 08:52 <sma...@outlook.de> wrote: > > > I've removed serialization stuff from the PR and rebased it to master > > https://github.com/apache/commons-csv/pull/276 > > > > kind regards, > > Markus > > > > From: Gary D. Gregory <ggreg...@apache.org> > > Sent: Friday, October 21, 2022 14:18 > > To: dev@commons.apache.org <dev@commons.apache.org> > > Subject: Re: [VOTE] Release Apache Commons CSV 1.10.0 based on RC1 > > > > On 2022/10/20 22:56:05 Alex Herbert wrote: > > > On Thu, 20 Oct 2022 at 23:43, Alex Herbert <alex.d.herb...@gmail.com> > > wrote: > > > > > > > > I did not have time to track through whether this behaviour changed > > > > after the initial implementation of the flag. I would think not as > the > > > > original behaviour is from 1.0. This would map to: > > > > > > > > true -> ALLOW_ALL > > > > false -> ALLOW_EMPTY > > > > new -> DISALLOW > > > > > > > > Which is what we currently have in 1.10.0 RC1. Thus the PR #276 [7] > to > > > > change the use of the flag to 'false -> DISALLOW' is not maintaining > > > > behavioural compatibility (to 1.7, or back to 1.0). > > > > > > PS. I just verified that PR 276 changes the DuplicateHeaderMode value > > > for allowDuplicates=false and does not change any tests. > > > > > > So the test suite is currently not enforcing behavioural > > > compatibility. This seems like a glaring hole in the tests and should > > > be addressed to prevent regressions. > > > > In git master, there are many tests for senders of > > withAllowDuplicateHeaderNames(boolean) and > > setAllowDuplicateHeaderNames(boolean). > > > > I filled in the coverage in testGetAllowDuplicateHeaderNames() for > > getAllowDuplicateHeaderNames() to reflect the _current_ behavior, right > or > > wrong. > > > > Let's reassess now git master now please. If the PR matters still, it > > should be rebased on git master. > > > > Gary > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
