Re: [PATCH] Issue #3919: fix for spurious property conflict during merge

Hi Stefan,

> thanks for this patch.
>
> I have one suggestion:
>
> [...]
>
> Instead of doing nothing, I think this should set the *state to 'merged'
> using
>
>        set_prop_merge_state(state, svn_wc_notify_state_merged);
>
> Just like apply_single_prop_add() already does.
>
> Do you agree? If so I will apply this patch with that modification.

Yes, that seems very reasonable since the other routines for merging
props (add/delete) call that function under similar circumstances (the
new value is the same as the working value).

> Also, did you already run the regression tests with your patch ("make
> check")? My suggestion might affect the output of 'svn' so tests would
> need to be run again (but I will run them either way before committing).

I did not.

BTW, I really have to hand it to you guys for putting together some
nice online docs that explains your development process and the
awesome Makefile.svn which allows a new developer to get started with
the code so quickly. I was wondering if you had unit/regression tests
but I did not see anything about that in the docs, although it is
quite possible I missed it.

Thank you very much!
-BN
Received on 2011-06-15 14:42:21 CEST