Code review comment for lp:~spiv/bzr/per-file-merge-hook-491711

Revision history for this message
Andrew Bennetts (spiv) wrote :

This addresses most (all?) of the points raised in the original reviews.

Major changes:

  * Added 'news_merge' plugin to contrib/! This is an example plugin using the merge hook function so that you can see it in action, and so that users will have some inspiration for writing their own.
  * The built-in merge logic is now invoked the same way as user-registered hook functions. So in theory anything the built-in logic can do, a plugin can do too.
  * the hook point is invoked a bit earlier, so it can have a say in more kinds of conflicts by inspecting the 'winner' attribute.
  * added some convenience methods/properties to the params object.
  * generally simpler and more linear code path than lp:bzr, thanks to refactorings for above changes.

I think looking at the news_merge plugin the API is still a bit cumbersome (note the repetition of logic vs. the built-in text merge to see if this is an ordinary regular file, both sides changed merge). But having an example proves it works, and gives us good data for thinking of improvements.

« Back to merge proposal