Daniel Näslund wrote:
> I have tried to follow your receipe. I couldn't find a way for switch to
> use several targets though. The second usage form "switch --relocate
> FROM TO [PATH...]" seems to only be about changing the metadata of the
> wc.
Oh yes, you're right.
Daniel Näslund wrote:
> Double gah! Still no patch, just the original message. I'm a mutt user.
> Could the new patch to tigris be the cause? Trying with a diff ending.
(The patch to Tigris was intended to fix problems in this area, I think.
I don't know if it was intended to fix this particular problem, or if it
might have caused it.)
> > Gah! No patch attached. Strange... Trying again.
> [[[
> Fix issue #3342: Summary of conflicts printed at end of up/sw/merge
> * subversion/svn/merge-cmd.c
> (svn_cl__merge): Call svn_cl__print_conflict_stats.
>
> * subversion/svn/cl.h
> (svn_cl__print_conflict_stats): Added declaration.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Call svn_cl__print_conflict_stats.
>
> * subversion/svn/switch-cmd.c
> (svn_cl__switch): Call svn_cl__print_conflict_stats.
>
> * subversion/svn_notify.c
> (svn_cl__print_conflict_stats): Changed name from print_conflict_stats.
>
> * subversion/svn_notify.c
> (notify): Remove references to print_conflict_stats. Do not clear
> counters for conflicts.
> ]]]
This looks great except for a few small things:
> Index: subversion/svn/update-cmd.c
> ==================================================================
>
> - return svn_client_update3(NULL, targets,
> + svn_error_t *err = svn_client_update3(NULL, targets,
> &(opt_state->start_revision),
> depth, depth_is_sticky,
> opt_state->ignore_externals,
> opt_state->force,
> ctx, pool);
We use plain C (C'89/90) in which you can't declare a new variable part
way through the code.
However, we don't need an 'err' variable here. If the call to
svn_client_update3() returns an error, we want to return that error
straight away, and the way to do that is to wrap the call in SVN_ERR:
SVN_ERR(svn_client_update3(...));
Also, please re-indent the follow-on lines to line up with the first
argument, "NULL".
> + svn_cl__print_conflict_stats(ctx->notify_baton2, pool);
This function can also return an error, so you must catch the error.
Again, use SVN_ERR(svn_cl__...(...)).
Of course, the same comments apply to other parts of the patch.
> +svn_error_t *
> +svn_cl__print_conflict_stats(void *notify_baton, apr_pool_t *pool)
> {
> + struct notify_baton *nb = (struct notify_baton*) notify_baton;
You don't need the explicit type cast.
> - if (nb->in_external)
> - {
> - nb->in_external = FALSE;
> - nb->ext_text_conflicts = nb->ext_prop_conflicts
> - = nb->ext_tree_conflicts = nb->ext_skipped_paths = 0;
> - if ((err = svn_cmdline_printf(pool, "\n")))
> - goto print_error;
> - }
You removed this code but I think most of it is still needed. I think
you should have removed only the part that resets the conflict counters.
> @@ -699,7 +678,6 @@
> svn_error_clear(err);
> }
>
> -
> void
> svn_cl__get_notifier(svn_wc_notify_func2_t *notify_func_p,
> void **notify_baton_p,
This is trivial, of course, but it's nicer if you don't include
irrelevant changes like this.
Finally, can you confirm that you have run the test suite ("make
check")? I don't know whether there are any tests that would detect this
change.
Stefan and Edmund are still thinking about moving this into a lower
layer, but I hope they will agree that it is fine to go ahead with your
patch in the meantime, as your patch won't make the move any more
difficult if we do decide to move it.
Thanks.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2377304
Received on 2009-07-31 11:15:50 CEST