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
