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
