Code review comment for lp:~jameinel/bzr-builddeb/changelog-hook

Revision history for this message
James Westby (james-w) wrote :

> This adds a per-file commit hook, which checks
> a) A Branch config called
> changelog_hook_enabled = True
> b) A file named 'changelog' or 'debian/changelog'
>
> If both hold true, and it perceives a conflict in the changelog file, it
> essentially runs Scott's merge_changelog code. This will always result in a
> conflict-free changelog file. The current algorithm is "prefer-mine". So if
> both changelogs have a section with a matching 'version', the text present in
> "this" are used.

That sounds wrong to me, why wouldn't it conflict those blocks?

> There are some new tests for this, but I've also manually tested merging
> lp:ubuntu/bzrtools @-r4.3.1 into -r 50. And it seemed to do a nice clean merge
> (without the plugin enabled it conflicted as expected.) As an example:
>
> bzr branch lp:ubuntu/bzrtools -r 4.3.1 bzrtools_upstream
> bzr branch lp:ubuntu/bzrtools -r 50 bzrtools_ubuntu
> cd bzrtools_ubuntu
> bzr merge ../bzrtools_upstream
>
> (There are other conflicts, but *debian/changelog* is not one of them)

Great, thanks for your work on this.

29 + merge.Merger.hooks.install_named_hook(
30 + 'merge_file_content', changelog_merge_hook_factory,
31 + 'Changelog file merge')

We should put "Debian" in there somewhere. I can do that on merge.

Other than that this looks fine to me. I want to discuss the conflict
issue before merging though.

Thanks,

James

review: Needs Information

« Back to merge proposal