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

Re: Fold the merge_automatic API into the existing merge_peg API

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 28 Mar 2013 21:07:44 +0000 (GMT)

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

This is an archived mail posted to the Subversion Dev mailing list.