Hi Noorul,
two small remarks below:
On Thu, Nov 18, 2010 at 04:51:50PM +0530, Noorul Islam K M wrote:
> Index: subversion/svn/log-cmd.c
> ===================================================================
> --- subversion/svn/log-cmd.c (revision 1036324)
> +++ subversion/svn/log-cmd.c (working copy)
> @@ -647,12 +647,12 @@
> target = APR_ARRAY_IDX(targets, i, const char *);
>
> if (svn_path_is_url(target) || target[0] == '/')
> - return svn_error_return(svn_error_createf(
> - SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> - _("Only relative paths can be
> specified"
> - " after a URL for 'svn log', "
> - "but '%s' is not a relative path"),
> - target));
> + return svn_error_createf(
> + SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
The above two lines could be one line, like this:
+ return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Only relative paths can be specified"
> + " after a URL for 'svn log', "
> + "but '%s' is not a relative path"),
> + target);
> }
> }
>
> Index: subversion/svn/relocate-cmd.c
> ===================================================================
> --- subversion/svn/relocate-cmd.c (revision 1036324)
> +++ subversion/svn/relocate-cmd.c (working copy)
> @@ -97,8 +97,20 @@
> apr_pool_t *subpool = svn_pool_create(scratch_pool);
> int i;
This part of the patch doesn't seem to be related to the rest.
It's not eliminating redundant svn_error_return() calls:
>
> + /* Target working copy root dir must be local. */
> for (i = 2; i < targets->nelts; i++)
> {
> + path = APR_ARRAY_IDX(targets, i, const char *);
> + if (svn_path_is_url(path))
> + return svn_error_return
> + (svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
Also, please don't put any whitespace (like spaces or newlines) between
a function name and the opening brace.
This would be the preferred style:
+ return svn_error_return(
+ svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR,
Thanks,
Stefan
> + NULL,
> + _("'%s' is not a local path"),
> + path));
> + }
> +
> + for (i = 2; i < targets->nelts; i++)
> + {
> svn_pool_clear(subpool);
> path = APR_ARRAY_IDX(targets, i, const char *);
> SVN_ERR(svn_client_relocate2(path, from, to, ignore_externals,
