[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