[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn_auth.h API change

From: <kfogel_at_collab.net>
Date: 2003-12-29 17:58:23 CET

Justin Erenkrantz <justin@erenkrantz.com> writes:
> > And Justin Erenkrantz, why did you vote +1 on this change (for 1.0)
> > even though it had doc bugs? :-)
>
> How are you testing for doc bugs? I don't have the tools installed to
> test for this. And, tools/examples isn't even in our build system.
> If you mean for this to be part of our validation, then we should add
> it to our build.

Huh? That question seems like a bit of a non sequitur. Just because
something can't be tested automatically doesn't mean it isn't a bug.
Isn't that exactly why human review is part of the process?

> > These reviews should be serious & thorough. Even Tobias hadn't voted
> > +1 on it yet (I don't know if that was an oversight, or if he had been
> > planning to review his own change again before voting.)
> >
> > That's all. I'm sure I'll make a review mistake at some point too;
> > and I hope I'll be held to account when I do! (By Justin, preferably.)
>
> I care more about correctness bugs. And, I looked at *every* line of
> the patch, confirmed that it was implemented in a sane manner without
> any problems that were evident to me. (I helped with the design of
> the auth system, so yes, I know how it should work.)

Whoa, slow up there, Tex :-). I wasn't accusing you of not knowing
how the auth system works, and you don't need to defend yourself about
that. Your technical knowledge was not in question here.

"I care more about correctness bugs"... I don't understand what that's
supposed to mean. Are you saying it's unfair to expect you to catch
doc bugs? That that's not supposed to be part of review? That people
only have to review for the things they personally care about, and the
project's coding standards don't matter? You know Subversion's doc
standards as well as anyone. A missing parameter is a correctness
issue in this project, and under other circumstances you'd be the
first to agree.

Sheesh, dude, just say "Whups, I made a mistake, sorry", correct it
[see below] and that's all. This isn't the worst thing in the world.
You're going out on a limb to defend the indefensible, for a
relatively minor matter too :-(.

> If Tobias didn't fix the docstrings, because I casted a +1 to it, I
> would fix it myself. But, he already did so in r8101. I honestly
> don't know what more you'd expect from volunteers. -- justin

There is one thing you could do:

Tobias added r8101 to the STATUS file, but not in the same item as
r8013. Meanwhile, your +1 is still on the latter, but not yet on the
former. You could review and vote on r8101, and also move it to be
with r8013, since they go together (though r8101's log message seems
not to have mentioned that it is partially a followup to 8013, not
sure why). Or, maybe undo your +1 on r8013 until you can vote on them
both?

Anyway, at the moment, you still have a +1 vote registered for a
change with doc bugs. (Of course, you're keeping track of the
situation now, so no doubt that's a short term situation -- would be
nice if the STATUS reflected what you know in your head, though.)

But my mail wasn't mainly about spurring you to take these specific
actions. I'm really asking you (and all of us) to be careful with
future reviews, that's all. If someone votes +1 on a patch that turns
out to have an unambiguous problem, then they should be made aware of
that, and I intend to do so whenever it happens, including to myself.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 29 18:51:30 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.