On Wed, Jan 07, 2004 at 12:55:09PM -0500, Greg Hudson wrote:
> On Wed, 2004-01-07 at 06:38, Greg Stein wrote:
> > By definition, a veto only applies to code changes (rather than, say,
> > process discussions or release decisions).
>
> Except in this case, the STATUS file says "A change needs three +1
> votes... and no vetoes, to go into the release." So the meaning of this
> particular veto is rather different from what you described.
Euh. This *is* about a code change (replace the parser). What I meant
above is that a person can't say "I veto making the 0.36 release now." By
its very nature, votes and vetoes control what goes *into* a release.
> > The current patch still seems to have a few open issues on the recognized
> > formats (e.g. cvs/rcs formats, or use of a slash). At this point in the
> > release, I'd hope we would have a more solid closure/consensus on the
> > formats before application.
>
> This is FUD.
Easy, please. That is a loaded word. My point was that discussion is
ongoing, rather than "done". As I said, I would hope to see a much more
solid "done" before adjusting the 1.0 branch. I do agree that the basic
code appears to be "done".
> We know we can add formats more easily than we can take
> them away, so mbk chose to start conservatively. Given that, it's
> positively expected that someone will identify a candidate to be added.
Sure.
> That doesn't translate into an "open issue" with the patch.
*shrug* I see it as an issue. If the patch was, "we discussed the right
set of formats over the past few months, and it now implements the
consensus", then I wouldn't see an issue. As it is, since the set of
formats is *not* "closed", then I see us applying further tweaks to this
area. And the notion of "further tweaks" bothers me for a branch that is
trying to *avoid* tweakage.
(yet I do agree that in concrete terms the "further tweaks" will be *very*
minimal risk exposure, especially compared to the initial introduction of
the new parser)
> > My second issue is that the patch is incomplete. I recognize that is
> > simply because it is meant as a "talking reference", but that is too loose
>...
> I don't think the patch was meant as a "talking reference." This is the
Karl asked mbk for an patch so that a concrete discussion could be
started. Otherwise, it was looking to be an abstract "yah. the current
parser sucks. so now what?" type of discussion.
> first indication I've seen that the patch is incomplete.
The docs are incomplete, although I know that has been noted and a patch
is being developed for that.
>...
> that. The lack of regression tests does not seem like it should be
> considered a show-stopper, since we have no such tests around getdate.y.
>
> ("But we have three years of experience around getdate.y" is an obvious
> answer, but it lacks perspective. People don't use dates much, so we're
> unlikely to have found out if people encountered weird corner-case
> behavior with getdate.y.)
We right regression tests to look for, well, regressions :-). In this
case, we want to ensure that when the new parser is put into place, that
the formats we expect will be parsed properly. And that the formats which
are illegal will properly generate errors. Since we also expect to add a
format or two, that will change the ordering of trying out the various
formats. It is possible that an alteration of the ordering will cause a
particular input to be picked up by a new pattern format and, thus, be
parsed as a different value.
We don't have any regression tests because the code is assumed to be
correct and we haven't made any changes to it. If somebody made a change,
then we might have wanted tests to ensure that the introduction of those
changes would not have changed the interpretation of (some set of) certain
formats.
At some point in the future, it might be nice to have regression tests as
a way to generate code coverage reports, and that would have (hopefully)
covered the various behaviors of getdate.y. But that is an ideal position
for quite another time.
>...
> Regardless of how important a piece of code is, we should try to ensure
> that it is correct and maintainable--if not now, then eventually. I am
Agreed.
> adamant about fixing the date parser before 1.0 (now that mbk has
> provided an avenue to do so) because if we don't fix it now, we have no
> sane road map towards making it correct or maintainable later. (As I
> noted before, I don't consider your road map sane, even a little bit.)
Obviously, we disagree here :-), and that is the nature of the voting
process.
>...
> Karl wrote:
> > It would be painful to discover a subtle date parsing bug right after
> > releasing 1.0.
>
> No, not really. An obscure date-parsing bug seems like the epitome of
> painlessness, as far as bugs go. We'd fix it in 1.0.1 and that would be
> that.
And then we'd need to get that rolled out...
> (Also, we're parsing a handful of date formats here, not C code. There
> just aren't all that many corner cases.)
Then let's see a regression suite to verify that.
As I said, I'm -0 on the whole concept here, which means that there is
room for resolution. Gathering the pieces together here, I think that
means the following things, at least, are missing:
* fix Windows build
* add regression suite to validate new date parser
* doc changes
* not a holdup, but needs to be done: adjust packages' build reqts to
eliminate bison
I'll happily re-review at that point, and if there aren't any further
problems, then I'll change my vote to a -0.
I'd still rather see us specifying a contract and adjusting the docs, but
a -0 means I won't get in the way of the new parser.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jan 7 20:08:52 2004