[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1355017 - in /subversion/trunk/subversion: libsvn_wc/conflicts.c libsvn_wc/conflicts.h tests/libsvn_wc/conflict-data-test.c

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 28 Jun 2012 13:24:22 -0400

On Thu, Jun 28, 2012 at 10:20 AM, <rhuijben_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/conflicts.c Thu Jun 28 14:20:15 2012
> @@ -90,6 +90,134 @@ svn_wc__conflict_skel_is_complete(svn_bo
>   return SVN_NO_ERROR;
>  }
>
> +/* Serialize a svn_wc_conflict_version_t before the existing data in skel */
> +static svn_error_t *
> +conflict__prepend_location(svn_skel_t *skel,
> +                           const svn_wc_conflict_version_t *location,
> +                           svn_boolean_t allow_NULL,
> +                           apr_pool_t *result_pool,
> +                           apr_pool_t *scratch_pool)
> +{
> +  svn_skel_t *loc;
> +  SVN_ERR_ASSERT(location || allow_NULL);
> +
> +  if (!location)
> +    {
> +      svn_skel__prepend(svn_skel__make_empty_list(result_pool), skel);
> +      return SVN_NO_ERROR;
> +    }
> +
> +  /* ("subversion" repos_root_url repos_uuid repos_relpath rev kind) */
> +  loc = svn_skel__make_empty_list(result_pool);
> +
> +  svn_skel__prepend_str(svn_node_kind_to_word(location->node_kind),
> +                        loc, result_pool);
> +
> +  svn_skel__prepend_int(location->peg_rev, loc, result_pool);
> +
> +  svn_skel__prepend_str(apr_pstrdup(result_pool, location->path_in_repos), loc,
> +                        result_pool);
> +
> +  if (!location->repos_uuid) /* Can theoretically be NULL */
> +    svn_skel__prepend(svn_skel__make_empty_list(result_pool), loc);

For NULL, you probably want "". Regardless, you need to insert the
UUID if it is present.

> +
> +  svn_skel__prepend_str(apr_pstrdup(result_pool, location->repos_url), loc,
> +                        result_pool);
> +
> +  svn_skel__prepend_str(SVN_WC__CONFLICT_SRC_SUBVERSION, loc, result_pool);

What is this "source" all about? You haven't documented this anywhere.

> +
> +  svn_skel__prepend(loc, skel);
> +  return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_wc__conflict_skel_set_op_update(svn_skel_t *conflict_skel,
> +                                    const svn_wc_conflict_version_t *original,
> +                                    apr_pool_t *result_pool,
> +                                    apr_pool_t *scratch_pool)
> +{
> +  svn_skel_t *why;
> +  svn_skel_t *origins;
> +
> +  SVN_ERR_ASSERT(conflict_skel
> +                 && conflict_skel->children
> +                 && conflict_skel->children->next
> +                 && !conflict_skel->children->next->is_atom);

How about a helper macro or some comment to explain what this is testing?

And I don't see any test for "It is an error to set another operation
to a conflict skel that already has an operation."

>...
> +svn_error_t *
> +svn_wc__conflict_skel_set_op_switch(svn_skel_t *conflict_skel,
> +                                    const svn_wc_conflict_version_t *original,
> +                                    apr_pool_t *result_pool,
> +                                    apr_pool_t *scratch_pool)
> +{
> +  svn_skel_t *why;
> +  svn_skel_t *origins;
> +
> +  SVN_ERR_ASSERT(conflict_skel
> +                 && conflict_skel->children
> +                 && conflict_skel->children->next
> +                 && !conflict_skel->children->next->is_atom);
> +

Similar comments here, and set_op_merge()

>...

Cheers,
-g
Received on 2012-06-28 19:24:55 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.