On Thursday 06 November 2003 23:21, kfogel@collab.net wrote:
> John Szakmeister <john@szakmeister.net> writes:
[snip]
> One minor question, in the new actions.py:run_and_verify_export():
> > + if isinstance(output_tree, wc.State):
> > + output_tree = output_tree.old_tree()
> > + if isinstance(disk_tree, wc.State):
> > + disk_tree = disk_tree.old_tree()
> > +
> > + # Remove dir if it's already there.
> > + main.safe_rmtree(wc_dir_name)
>
> You already remove the target in the caller. Are you sure you want to
> remove it here? We might someday have a test that tests what happens
> when you export over an existing directory, for example.
>
> I realize that you're probably copying from run_and_verify_checkout(),
> which does the same thing, and I have no idea why it does. Especially
> since it's doc string doesn't mention the behavior :-).
>
> I read mail on a different machine than my working copy, but will
> apply this in the morning.
Uh, well, I cheated and borrowed from the run_and_verify_export(). :-) I
actually wondered about that a bit myself, but figured there must have been
some reason for it... so I left the call there. I don't see any harm in
removing it.
I'll pull that portion out and re-submit the patch later tonight.
Thanks for the review!
-John
> Thanks!
>
> -Karl
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 7 11:36:23 2003