Re: svn commit: rev 1562 - trunk/subversion/include trunk/subversion/libsvn_wc trunk/subversion/libsvn_subr trunk/subversion/libsvn_client trunk/subversion/svnlook trunk/subversion/libsvn_repos

On Tue, Mar 19, 2002 at 09:35:27PM -0600, Karl Fogel wrote:
> Greg Stein <gstein@lyra.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
> out?

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
replacement.

> `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"

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