Tree Transform passing all tests, plus abuse

Aaron Bentley aaron.bentley at utoronto.ca
Mon Feb 13 18:30:03 GMT 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I've uploaded a new version of Tree Transform, with many of these issues
addressed.  In particular, win32 support has been tested and fixed.

John A Meinel wrote:
> In TreeTransform.__init__() you use a plain os.mkdir(), we should
> probably trap for some sort of exception, at least so we can raise a
> more informative one.

This is done.

> 
> You do the same thing in finalize(), where you don't handle any
> exceptions. This may be fine, just wanted to bring it up.

I'm now throwing an informative exception.

> There are some PEP8 issues in transform.py

I think they're fixed.  Please let me know if I've missed some.

> In test_transform.py you Branch.initialize and then chdir('..'). I think
> it would be better if we create a subdirectory branch, rather than
> changing directory outside of the Test working directory.

Done.

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

I still don't have a good idea what changes to make here.

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

Changed.

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

I haven't changed this.  I'm not sure about the wisdom of using
non-native path separators.  I believe this is a very different case
from using '\' in transports.

Is there a reason not to use the native path separators?

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

I've yanked all the changes that originated in bzr.ab and weren't merged
in your integration branch.

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

I went with sys.platform != 'win32'.

So I hope the remaining issues are
1. some method names could be improved
2. uses native path separators

Is there anything else?

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFD8NAr0F+nu1YWqI0RApHyAJ4j7J8O3JfTnr3Q5oC7SATmcz5vtwCfbyfG
eXkjQ/0+mJTxp+WTR73KG10=
=2xaT
-----END PGP SIGNATURE-----




More information about the bazaar mailing list