[MERGE REVIEW] remerge correctly handles single-file arguments
Martin Pool
mbp at sourcefrog.net
Mon Feb 27 22:24:05 GMT 2006
On 27 Feb 2006, Aaron Bentley <aaron.bentley at utoronto.ca> wrote:
> Hi all,
>
> The Tree Transform work contained a regression affecting remerge; we
> lost the ability to specify particular files to re-merge.
>
> This patch corrects this, and also throws NotVersioned if requested to
> remerge an unversioned or non-existant file.
+1, looks reasonable to me.
(It occurs to me while looking at this that a major role for reviewers
in our process is to make sure that the newly added tests correspond to
the stated purpose of the merge, and that the code changes correspond to
the tests.)
> === modified file 'bzrlib/merge.py'
> --- bzrlib/merge.py
> +++ bzrlib/merge.py
> @@ -227,7 +227,8 @@
>
> def do_merge(self):
> kwargs = {'working_tree':self.this_tree, 'this_tree': self.this_tree,
> - 'other_tree': self.other_tree}
> + 'other_tree': self.other_tree,
> + 'interesting_ids': self.interesting_ids}
> if self.merge_type.requires_base:
> kwargs['base_tree'] = self.base_tree
> if self.merge_type.supports_reprocess:
> @@ -318,7 +319,8 @@
> history_based = False
>
> def __init__(self, working_tree, this_tree, base_tree, other_tree,
> - reprocess=False, show_base=False, pb=DummyProgress()):
> + interesting_ids=None, reprocess=False, show_base=False,
> + pb=DummyProgress()):
> """Initialize the merger object and perform the merge."""
> object.__init__(self)
> self.this_tree = working_tree
(I realize it's existing code so this comment is not aimed at this
merge.)
This pattern with the kwargs and very long argument list suggests that
maybe we should create a Merger object and set values into it, before firing
it off, rather than setting everything in the constructor.
Btw it seems wierd to me that Merge3Merger doesn't inherit from Merger.
Why is that?
--
Martin
More information about the bazaar
mailing list