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

Re: svn commit: r30931 - trunk/subversion/libsvn_client

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Fri, 02 May 2008 11:07:14 -0400

kameshj_at_tigris.org writes:
> Log:
> Prior to this Fix,
> '/tmp' *not* being a working copy, following command executed from '/tmp'
> and /tmp/abc having some useful contents, would wipe off /tmp/abc.
> $svn mkdir --parents abc
>
> As a part of this fix we delete '/tmp/abc' upon failure only if this command
> has created it.

Thanks, Kamesh, nice catch.

I had to read the log message a couple of times to get it. A few ways
to improve it:

   - Put all the conditions together. That is, the reproduction
     scenario involves two preconditions: "/tmp is not a working copy"
     and "/tmp/abc has some useful contents". State those two together,
     at the front of the recipe.

   - Don't bother to specify whether /tmp/abc's contents are "useful" or
     not. That's not relevant to the recipe; the point is that it will
     be deleted when it shouldn't be. Whether its contents were
     valuable to the user or not is independent of the Subversion bug.

   - State whether /tmp/abc is a file or directory :-).

So, for example:

   Prior to this fix, if '/tmp' were *not* a working copy and
   /tmp/abc existed (as either a file or directory), then running
   'svn mkdir --parents abc' from within '/tmp' would delete
   '/tmp/abc'. The fix is to delete '/tmp/abc' upon failure only if
   this command had created it in the first place.

But most importantly: this whole recipe really belongs in a comment in
the code, rather than the log message. It's readers of the code who
will want to know what the kind==svn_node_node check is for.

Best,
-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-02 17:07:30 CEST

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