Julian Foad wrote:
> Hyrum K. Wright wrote:
>
>> This patch comprises the first step in moving multiple files, namely
>> revving the client library API, and modifying the client application to
>> call this new function. More patches are on their way, but I figure it
>> is better to submit small changes at a time in separate patches.
>
>
> It is a good idea to send these patches for review in small stages.
> However, we should not actually commit this patch as it is, because the
> result would be an API whose doc-string is inconsistent with its
> implementation. (I suppose you could make the API say "Exactly one
> source must be supplied", and then this would be a self-contained,
> logical, self-consistent change, but this isn't a sufficient reason on
> its own for revving the API.)
>
> When a good part of the underlying implementation for it is ready, then
> we can commit this, as the added functionality will be sufficient reason
> for revving the API. Probably, by that time, you will have found some
> reason to change this part slightly.
Good point, I'll hold off on exactly what needs to be done with the API
until I've worked a bit more on the implementation.
> I don't want "copy" to remain only able to handle a single file while
> "move" gets the ability to handle multiple files. I feel it would be
> neater to make the corresponding changes to "copy" at the same time as
> "move". However, that could involve twice as much re-work after
> reviews, so perhaps it makes sense to submit a patch for just "move" and
> then, when it is approved, update "copy" in the same way.
I think that is will be easier to first implement move, which only has
two cases (WC->WC and URL->URL), and then go back and implement copy,
which has four.
> Presumably, for multiple source paths, the destination must be a
> directory into which the sources will be moved, whereas for a single
> item it can be the new name for the item. (This part of the doc string,
> about "dst_path", is already ambiguous; I've just posted a patch to
> clarify it.)
>
> I think we might well need to use two separate functions, one for moving
> (and possibly renaming) a single item, and one for moving multiple (one
> or more) items without renaming them. Otherwise, if only one source
> item is specified, how do we know whether the destination path or url
> represents the new _name_ for the item or the new _directory_ in which
> to place it?
Wouldn't it be possible to just check and see if the destination is a
target when there is more than one source? If that case were not met,
we could error.
> Basically your code looks OK. As I said, it's not appropriate to commit
> it yet, and the API probably needs some work. Please (all readers) have
> a look at the thread "API proposal - issue 2188 - svn_client_copy/move"
> where I recently proposed a new single-file copy/move API. Some aspects
> of the proposal may be too complex to implement all at once; I'm not
> sure. Anyway I think the way forward is to formulate a multi-file
> copy/move API based on the thinking in that proposal, so that they have
> most semantics in common.
>
> I'll look into the API definition; Hyrum, you can continue working on
> the lower-level parts that will be needed, unless you feel that
> splitting the API later into two different functions would invalidate
> much of your work.
I think that I'll be able to continue just fine. I look forward to your
findings about the API definition.
-Hyrum
Received on Thu Nov 17 23:58:33 2005