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.
- Julian
Received on 2013-03-28 22:08:18 CET