Re: Fold the merge_automatic API into the existing merge_peg API

On 28.03.2013 22:07, Julian Foad wrote:
> Branko Čibej wrote:
>
>> On 28.03.2013 21:38, Julian Foad wrote:
>>> I like the focused API, but I also see how the automatic merge can be
>>> considered to fill in a bit of missing functionality that merge_peg ought to
>>> provide.
>>>
>>> Perhaps we can have both. Teach merge_peg to DTRT in this case, and still
>>> have the focused API available for when a client knows it wants an automatic
>>> merge. Is there sufficient merit in that to outweigh the overhead of having to
>>> test two similar but different entry points?
>>>
>>> The attached patch moves the decision to call the 'automatic merge'
>>> API from 'svn' into 'svn_merge_peg5()'. I have run some merge
>>> tests and tree conflict tests, but not the whole test suite yet. Here is the
>>> log msg.
>>>
>>> [[[
>>> Teach svn_client_merge_peg5() to do an automatic merge, and let 'svn merge'
>>> rely on that instead of calling the dedicated automatic merge APIs.
>>>
>>> TODO: Decide whether to keep or make private the 'automatic merge' APIs.
>>> TODO: This reduces the verbosity of 'svn merge --verbose'. Consider
>>> doing something about it, perhaps by adding some new notifications for the
>>> notifier callback?
>>>
>>> * subversion/include/svn_client.h,
>>> subversion/libsvn_client/merge.c
>>> (svn_client_merge_peg5): Do an automatic merge if no revision range
>>> specified.
>>>
>>> * subversion/svn/merge-cmd.c
>>> (automatic_merge): Delete.
>>> (run_merge): Don't take special action to handle an automatic merge, let
>>> the pegged merge code path handle it.
>>> ]]]
>>>
>>> Thoughts?
>> I like it. Apparently the encapsulation is even simpler than I expected.
> Heads up: that patch is broken. merge_automatic_tests.py 7 though 14 all fail. However, it's most likely broken in a rather trivial way so I expect the corrected version will still be simple.
>
>> For JavaHL, a simple overload of ISVNClient.merge can provide the
>> "focused" interface without inventing yet another type of merge API.
>> Even better, passing null for the revision ranges in the existing
>> merge-peg overload can be made to yield the same effect, without
>> affecting the API signature at all. (Currently IIUC passing a null
>> ranges array will cause an error.)
> Making the C API accept NULL for the revision-ranges array argument would be a totally sensible extension. I'll do that.

I was thinking of the JavaHL API, but you're right -- the C API could
benefit from the same simplification.

-- Brane

-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com