You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/08/22 23:24:20 UTC

Re: svn commit: r32615 - branches/issue-2897-take2/subversion/libsvn_fs_base

kameshj@tigris.org writes:
> Log:
> On issue-2897-take2 branch:
>
> Attempt to compute 'mergeinfo_delta' only for 'svn:mergeinfo' property.
>
> * subversion/libsvn_fs_base/tree.c
>   (txn_body_change_node_prop): Attempt to compute 'mergeinfo_delta'
>    only for 'svn:mergeinfo' property.

Change looks good.  In log messages, it's better not to repeat the same
thing twice.  You could either leave out the introductory line, or write
something different in the function part (like "Check whether this is
the 'svn:mergeinfo' property before proceeding.").  Saying the exact
same thing twice causes the reader's brain to skip a beat, though :-).

-Karl

> Modified:
>    branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c
>
> Modified: branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c?pathrev=32615&r1=32614&r2=32615
> ==============================================================================
> --- branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c	Thu Aug 21 06:03:10 2008	(r32614)
> +++ branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c	Thu Aug 21 06:52:15 2008	(r32615)
> @@ -1309,13 +1309,14 @@ txn_body_change_node_prop(void *baton,
>    if (! proplist)
>      {
>        proplist = apr_hash_make(trail->pool);
> -      SVN_ERR(svn_mergeinfo_parse(&mergeinfo_added,
> -                                  args->value->data, trail->pool));
> +      if (strcmp(args->name, SVN_PROP_MERGEINFO) == 0)
> +        SVN_ERR(svn_mergeinfo_parse(&mergeinfo_added,
> +                                    args->value->data, trail->pool));
>      }
>    else
>      {
>        svn_mergeinfo_t deleted, orig_mergeinfo, new_mergeinfo;
> -      if (args->value)
> +      if (args->value && (strcmp(args->name, SVN_PROP_MERGEINFO) == 0))
>          {
>            svn_string_t *orig_mergeinfo_str = apr_hash_get(proplist,
>                                                            SVN_PROP_MERGEINFO,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: svn commit: r32615 - branches/issue-2897-take2/subversion/libsvn_fs_base

Posted by Kamesh Jayachandran <ka...@collab.net>.
Karl,

Fixed the log message

Thanks for the review
With regards
Kamesh Jayachandran
-----Original Message-----
From: Karl Fogel [mailto:kfogel@red-bean.com]
Sent: Sat 8/23/2008 4:54 AM
To: dev@subversion.tigris.org
Cc: kameshj@tigris.org
Subject: Re: svn commit: r32615 - branches/issue-2897-take2/subversion/libsvn_fs_base
 
kameshj@tigris.org writes:
> Log:
> On issue-2897-take2 branch:
>
> Attempt to compute 'mergeinfo_delta' only for 'svn:mergeinfo' property.
>
> * subversion/libsvn_fs_base/tree.c
>   (txn_body_change_node_prop): Attempt to compute 'mergeinfo_delta'
>    only for 'svn:mergeinfo' property.

Change looks good.  In log messages, it's better not to repeat the same
thing twice.  You could either leave out the introductory line, or write
something different in the function part (like "Check whether this is
the 'svn:mergeinfo' property before proceeding.").  Saying the exact
same thing twice causes the reader's brain to skip a beat, though :-).

-Karl

> Modified:
>    branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c
>
> Modified: branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c
> URL: http://svn.collab.net/viewvc/svn/branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c?pathrev=32615&r1=32614&r2=32615
> ==============================================================================
> --- branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c	Thu Aug 21 06:03:10 2008	(r32614)
> +++ branches/issue-2897-take2/subversion/libsvn_fs_base/tree.c	Thu Aug 21 06:52:15 2008	(r32615)
> @@ -1309,13 +1309,14 @@ txn_body_change_node_prop(void *baton,
>    if (! proplist)
>      {
>        proplist = apr_hash_make(trail->pool);
> -      SVN_ERR(svn_mergeinfo_parse(&mergeinfo_added,
> -                                  args->value->data, trail->pool));
> +      if (strcmp(args->name, SVN_PROP_MERGEINFO) == 0)
> +        SVN_ERR(svn_mergeinfo_parse(&mergeinfo_added,
> +                                    args->value->data, trail->pool));
>      }
>    else
>      {
>        svn_mergeinfo_t deleted, orig_mergeinfo, new_mergeinfo;
> -      if (args->value)
> +      if (args->value && (strcmp(args->name, SVN_PROP_MERGEINFO) == 0))
>          {
>            svn_string_t *orig_mergeinfo_str = apr_hash_get(proplist,
>                                                            SVN_PROP_MERGEINFO,
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org