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 <[email protected]> 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 <[email protected]> > Sent: Friday, October 21, 2022 14:18 > To: [email protected] <[email protected]> > 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 <[email protected]> > 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: [email protected] > > For additional commands, e-mail: [email protected] > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
