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
