[merge] Make merge internals private

John Arbash Meinel john at arbash-meinel.com
Wed Jan 18 00:16:09 GMT 2006


Aaron Bentley wrote:
> Hi all,
> 
> I'd like to replace the merge internals with the Transactional Tree
> Transform code when it's properly baked.  In order to avoid breaking API
> compatibility later, Robert Collins suggested I make the internals
> private now.  So I've done that.
> 
> http://panoramicfeedback.com/opensource/bzr/bzr.ab/
> 
> P.S. I've started work on merging via Transactional Tree Transform.  So
> far, so good.
> 
> Aaron

I think it is nice to have stuff made private. I just want to review the
code a bit, and make sure that we are making the right thing private.

Why is only MergConflictHandler moved into _merge.py, and not the Merger
class? Shouldn't something like 'merge_inner' be private?

I know I feel the merge() interface is still sub-optimal. merge_inner()
might be meant to be the internal interface, but it has different
side-effects than merge().
I think that barring exceptional conditions, the only place that
instantiates a Branch object should be at the UI level.
(builtins.py/commands.py which should be moved into bzrlib/ui/ anyway :)

Also, there are some PEP8 violations in both merge.py and _merge.py,
plus there is a comment about merge() which shouldn't be in _merge.py
(it probably can just be deleted).

If this stuff is going to be removed anyway, I'm not too concerned with
it being non-conformant.

It seems correct that merge_core.py and changeset.py should be made private.

Should 'get_tree' and 'get_revid_tree' be public?

The function 'file_exists()' doesn't seem to be used.

I'm not sure about build_working_dir, either. But transform_tree seems
useful.

Other than the above comments, the changes seem good.

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060117/de5ea228/attachment.pgp 


More information about the bazaar mailing list