Code review comment for lp:~spiv/bzr/better-news-merge

On Sat, 2010-02-13 at 01:30 +0000, Andrew Bennetts wrote:
>
> magic_marker = '|NEWS-MERGE-MAGIC-MARKER|'
>
> +# The order sections are supposed to appear in. See the template at
> the
> +# bottom of NEWS. None is a placeholder for an unseen section
> heading.
> +canonical_section_order = [
> + None, 'Compatibility Breaks', 'New Features', 'Bug Fixes',
> 'Improvements',
> + 'Documentation', 'API Changes', 'Internals', 'Testing']

This is duplicated with the template; perhaps you could use the template
instead? That would make this usable by other projects.
...
> + # Are all the conflicting lines bullets or sections?
> If so, we
> + # can merge this.
> + try:
> + base_sections =
> munged_lines_to_section_dict(base)
> + a_sections = munged_lines_to_section_dict(a)
> + b_sections = munged_lines_to_section_dict(b)
> + except MergeTooHard:
> + # Something else :(
> + # Maybe the default merge can cope.
> + trace.mutter('news_merge giving up')
> + return 'not_applicable', None

In the NEWS entry you aren't entirely clear about the implications of
'using bzr's builtin merge'. .. from the code it looks like 'if there
are conflicts outside the structured section none of the news file is
smart merged'. Perhaps we could merge just the non-section data with
bzr's built in merge, or make the NEWS entry clearer.
...
> # Transform the merged elements back into real blocks of
> lines.
> + trace.mutter('news_merge giving up')
> return 'success', list(fakelines_to_blocks(result_lines))

This mutter seems...wrong.

 review: needsfixing