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