Tree Transform passing all tests, plus abuse

John A Meinel john at arbash-meinel.com
Fri Feb 10 23:42:01 GMT 2006


Aaron Bentley wrote:
> Hi all,
> 
> The Tree Transform code is now passing all tests.  That includes the
> previously changeset-oriented test_mege_core tests, which I've now
> ported over to it.
> 
> I have also subjected it to abusive merges, (like merge -r 5..100 to
> bzr.dev), and added tests and fixes for the problems this revealed.  It
> now handles these abusive cases better than the existing merge code.
> 
> I think it is pretty much ready for prime time.
> 
> Tree transform features
> 
> * A simpler API than the one it replaces
> * Easy extension with new merge types
> * Separate functions for building, reverting and merging trees (instead
> of just using merge)
> * Ready for better conflict reporting (to be added when the code for new
> formats lands)
> * Up-front detection of errors and conflicts, so that operations don't
> die halfway through
> * Quite a lot of unit tests
> 
> Tree transform can be found here:
> http://code.aaronbentley.com/bzr/bzr.ttransform/
> 
> Please feel free to beat on it.

A couple of more code statements...

Some of the function names on TreeTransform aren't quite intuitive.
'get_id_tree' and 'get_trans_id', 'get_tree_file_id', etc

I also think it isn't quite clear when you would use "create_path"
versus "create_file"

I don't have a great suggestion, but I know it was a little bit
difficult to read test_transform.


The documentation for final_kind() (and a few other functions) uses """\
rather than putting the documentation on the same line.

Internally you use "os.path.join" rather than "osutils.pathjoin". Which
means we would end up with '\' separated paths.

The changes include your "defaults" changes. I like the idea of
defaults, but I don't know if the changes have been reviewed.


You fixed a lot of PEP8 in errors.py, and added a couple more errors:)

For 'supports executable' should we use os.name != 'nt', or sys.platform
== 'win32'

I don't know if win9x are supported platforms, but supports_executable()
would say that they do support the execute bit (when they don't).


All this said, the actual code changes themselves look pretty good. And
the proof is that the code works and does what we want. :)

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060210/7c02004a/attachment.pgp 


More information about the bazaar mailing list