Gareth McCaughan <gareth.mccaughan@pobox.com> writes:
> A first pass at a patch is attached (rather than included,
> because I have reason to suspect that my MUA sometimes
> wraps lines: sorry). Things I'm not entirely happy about:
I'm reviewing your comments, not your patch. Just so you know.
> 1. The new svn_client_revert_list function never returns
> an error when an entry doesn't exist. That's the behaviour
> svn_cl__revert (currently its only caller) wants, but it's
> not necessarily what every imaginable caller would want.
> Should there be another flag passed in to determine
> whether nonexistent entries should be treated just like
> other errors?
Hm... I really don't think we need both svn_client_revert() and
svn_client_revert_list(). Just make svn_client_revert() *not* take a
const char * path, and start taking an apr_array_header_t * targets
list.
> 2. The old svn_cl__revert used to call svn_cl__check_cancel
> after each individual reversion. It's not so easy to do that
> down in the depths, and in any case there's less need now
> that we aren't sleeping for 1s per reverted path. But, still,
> it makes me uneasy not to have the check there at all for
> an operation that can in principle take an unbounded amount
> of time. Should there be a callback or something?
svn_client_revert() should be handed a 'ctx' client callback back,
from which it should be able to call ctx->cancel_func(ctx->cancel_baton).
> 3. Is "svn_client_revert_list" the best name for the new function?
> Is it right for both this and the old function that reverts a single
> path to continue to exist? (I think the answer to the latter question
> is "yes", but am open to persuasion.)
I answered this already. Also, I'd prefer not to see a function named
svn_client__revert1() -- first of all, the "1" is just ... ugly. But
also, if this function is never called from outside of revert.c, we
need to drop the "svn_client__" prefix (which implies intra-module
globalness).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 1 02:25:51 2003