On Tue, Mar 19, 2002 at 09:35:27PM -0600, Karl Fogel wrote:
> Greg Stein <firstname.lastname@example.org> writes:
> > > `npath' is used in other places in that function (and not just for
> > > npath->data). Sure, they could all be changed, but... See above. :)
> > They should be changed. Come on, Karl. I did an inspection and saw that
> > 'npath' wasn't needed. And it was pretty clear that it wasn't used. If you
> > want to challenge even the most obvious changes, then why should I continue
> > with suggestions?
> Huh? How is this change different from the other stuff you pointed
Well, the change is awfully simple -- it is a simple replacement of 'npath'
with 'path'. It got a bit peeved that it fell into your bucket of "well,
that variable is actually used elsewhere, so it should stay," and that it
seemed you hadn't even taken a moment to actually look at the code which I
commented on. If you had, you would see it was just that: a simple
> `npath' is used as an argument to a recursive call (this is
> svn_wc__ensure_directory). So if I change it, I have to change the
> prototype of that function, which means changing all the other callers
> of that function, which is precisely the sort of change propagation I
> wanted to avoid (an avoidance with which you agreed). What am I
> missing here?
You're missing what my original comment was:
"The 'npath' variable isn't needed. Just use the 'path' parameter."
I didn't suggest a prototype change.
> > I did an inspection and saw that 'npath' wasn't needed. And it was
> > pretty clear that it wasn't used.
> But at any rate, it's not an accurate description of the code.
> There's nothing "obvious" about this change. It has the same costs as
> the other changes we're discussing. So what's up? :)
You lost my original comment, and substituted "use a 'const char *' instead"
Greg Stein, http://www.lyra.org/
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Wed Mar 20 06:51:19 2002