You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Malcolm Rowe <ma...@farside.org.uk> on 2006/12/08 18:21:41 UTC
Re: svn commit: r22612 - branches/multiple-moves/subversion/include
Hi Hyrum,
It's great that you're working on this! A few more comments about the
API, though.
On Fri, Dec 08, 2006 at 09:09:20AM -0800, hwright@tigris.org wrote:
> ==============================================================================
> --- branches/multiple-moves/subversion/include/svn_client.h (original)
> +++ branches/multiple-moves/subversion/include/svn_client.h Fri Dec 8 09:09:19 2006
> @@ -1905,15 +1905,24 @@
> *
> * @a src_paths must be files or directories under version control, or
> * URLs of a versioned item in the repository. If @a src_paths has multiple
> + * items, they must be all repository URLs or all working copy paths.
> + * @a src_revision is used to choose the revision from which to copy the
> + * @a src_paths.
> + *
> + * The parent of @a dst_path must already exist.
> + *
> + * If @a src_paths has only one item, attempt to move it to @a dst_path. If
> + * @a copy_as_child is TRUE and @a dst_path already exists, attempt to copy the
> + * item as a child of @a dst_path. If a child of @a dst_path already exists
> + * with the same name as the item in @a src_paths, fail with
> + * @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a working copy path and
> + * @c SVN_ERR_FS_ALREADY_EXSISTS if @a dst_path is a URL.
Typo: EXSISTS (also occurs later). Shouldn't that be 'attempt to copy'
rather than 'attempt to move'?
What happens if copy_as_child is FALSE?
Also, I'm not sure whether the third sentence ("If a child of @a
dst_path...") is still part of the 'If copy_as_child is TRUE as dst_path
already exists' clause or whether it's supposed to apply regardless.
> + *
> + * If @a src_paths has multiple items, @a copy_as_child is ignored, and all
> + * @a src_paths are copied as children of @a dst_path. If any child of
> + * @a dst_path already exists with the same name any item in @a src_paths,
> + * fail with @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a working copy path and
> + * @c SVN_ERR_FS_ALREADY_EXSISTS if @a dst_path is a URL.
> *
> * If @a dst_path is a URL, use the authentication baton
> * in @a ctx and @a ctx->log_msg_func/@a ctx->log_msg_baton to immediately
> @@ -1945,10 +1954,10 @@
> apr_pool_t *pool);
>
> /**
> + * Similar to svn_client_copy4(), with the difference that if
> + * @a dst_path exists, fail with @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a
> + * working copy path and @c SVN_ERR_FS_ALREADY_EXISTS if @a dst_path is a
> + * repository URL. Also, only take a single @a src_path and @a dst_path.
Isn't this the _same_ failing behaviour as svn_client_copy4()?
Should this just be something like 'Similar to svn_client_copy4(), with
only one @a src_path and @a copy_as_child set to FALSE'?
Similar comments also apply to the _move() implementation, of course.
Regards,
Malcolm
Re: svn commit: r22612 - branches/multiple-moves/subversion/include
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Malcolm Rowe wrote:
> Hi Hyrum,
>
> It's great that you're working on this! A few more comments about the
> API, though.
Malcolm,
Thanks for the extra pair of eyes, I've updated the docs in r22619,
hopefully addressing your concerns.
-Hyrum
> On Fri, Dec 08, 2006 at 09:09:20AM -0800, hwright@tigris.org wrote:
>> ==============================================================================
>> --- branches/multiple-moves/subversion/include/svn_client.h (original)
>> +++ branches/multiple-moves/subversion/include/svn_client.h Fri Dec 8 09:09:19 2006
>> @@ -1905,15 +1905,24 @@
>> *
>> * @a src_paths must be files or directories under version control, or
>> * URLs of a versioned item in the repository. If @a src_paths has multiple
>> + * items, they must be all repository URLs or all working copy paths.
>> + * @a src_revision is used to choose the revision from which to copy the
>> + * @a src_paths.
>> + *
>> + * The parent of @a dst_path must already exist.
>> + *
>> + * If @a src_paths has only one item, attempt to move it to @a dst_path. If
>> + * @a copy_as_child is TRUE and @a dst_path already exists, attempt to copy the
>> + * item as a child of @a dst_path. If a child of @a dst_path already exists
>> + * with the same name as the item in @a src_paths, fail with
>> + * @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a working copy path and
>> + * @c SVN_ERR_FS_ALREADY_EXSISTS if @a dst_path is a URL.
>
> Typo: EXSISTS (also occurs later). Shouldn't that be 'attempt to copy'
> rather than 'attempt to move'?
>
> What happens if copy_as_child is FALSE?
>
> Also, I'm not sure whether the third sentence ("If a child of @a
> dst_path...") is still part of the 'If copy_as_child is TRUE as dst_path
> already exists' clause or whether it's supposed to apply regardless.
>
>> + *
>> + * If @a src_paths has multiple items, @a copy_as_child is ignored, and all
>> + * @a src_paths are copied as children of @a dst_path. If any child of
>> + * @a dst_path already exists with the same name any item in @a src_paths,
>> + * fail with @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a working copy path and
>> + * @c SVN_ERR_FS_ALREADY_EXSISTS if @a dst_path is a URL.
>> *
>> * If @a dst_path is a URL, use the authentication baton
>> * in @a ctx and @a ctx->log_msg_func/@a ctx->log_msg_baton to immediately
>> @@ -1945,10 +1954,10 @@
>> apr_pool_t *pool);
>>
>> /**
>> + * Similar to svn_client_copy4(), with the difference that if
>> + * @a dst_path exists, fail with @c SVN_ERR_ENTRY_EXISTS if @a dst_path is a
>> + * working copy path and @c SVN_ERR_FS_ALREADY_EXISTS if @a dst_path is a
>> + * repository URL. Also, only take a single @a src_path and @a dst_path.
>
> Isn't this the _same_ failing behaviour as svn_client_copy4()?
> Should this just be something like 'Similar to svn_client_copy4(), with
> only one @a src_path and @a copy_as_child set to FALSE'?
>
> Similar comments also apply to the _move() implementation, of course.
>
> Regards,
> Malcolm