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

Re: svn commit: r27180 - in trunk/subversion: include libsvn_client libsvn_wc svn

From: Eric Gillespie <epg_at_google.com>
Date: 2007-10-16 04:20:32 CEST

sussman@tigris.org writes:

> Author: sussman
> Date: Sun Oct 14 14:11:29 2007
> New Revision: 27180

Looks good! A few minor comments:

> * subversion/svn/conflict-callbacks.c
> (print_conflict_description): remove crufty old debugging function.
> (svn_cl__ignore_conflicts): update function signature and use of API.

svn_cl__ignore_conflicts is never used; is it also crufty old?

> +++ trunk/subversion/libsvn_wc/merge.c Sun Oct 14 14:11:29 2007
> @@ -419,13 +419,23 @@
> cdesc.their_file = right;
> cdesc.my_file = tmp_target;
> cdesc.merged_file = result_target;
> + cdesc.property_name = NULL;
>
> SVN_ERR(conflict_func(&result, &cdesc, conflict_baton, pool));
> - switch (result)
> + if (result == NULL)
> + {
> + /* If we were harsh, we'd throw an error here.
> + Instead, we'll just pretend they want to postpone
> + the conflict resolution. */
> + result = svn_wc_create_conflict_result(
> + svn_wc_conflict_choose_postpone, NULL, pool);
> + }

Ugh, either raise the error or document this behavior in the doc
string. I vote for the former. Again later in this function, in
the binary-handling version.

> @@ -684,13 +706,33 @@
> user's file, we do nothing. We also do nothing if
> the response claims to have already resolved the
> problem.*/
> - case svn_wc_conflict_result_resolved:
> - case svn_wc_conflict_result_choose_mine:
> + case svn_wc_conflict_choose_mine:
> {
> *merge_outcome = svn_wc_merge_merged;
> contains_conflicts = FALSE;
> goto merge_complete;
> }
> + case svn_wc_conflict_choose_merged:
> + {
> + if (! result->merged_file)
> + {
> + /* ?? Callback asked us to choose its own
> + merged file, but didn't provide one? */
> + contains_conflicts = TRUE;

Hmm. I'm inclined to call this an API violation as well, and
raise an error.

> +svn_wc_conflict_result_t *
> +svn_wc_create_conflict_result(svn_wc_conflict_choice_t choice,
> + const char *merged_file,
> + apr_pool_t *pool)
> +{
> + svn_wc_conflict_result_t *result = apr_pcalloc(pool, sizeof(*result));

You explicitly initialize the only two fields, so just use palloc
instead of pcalloc.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Oct 16 04:21:05 2007

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.