[MERGE (0.12)] Bundle format 0.9

Aaron Bentley aaron.bentley at utoronto.ca
Tue Sep 26 14:46:32 BST 2006


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

John Arbash Meinel wrote:
> Aaron Bentley wrote:
> So before we can merge this, we need to decide as a group if we want the
> 'parent_check' patch, and I haven't heard anything from Robert or Martin
> yet.

I've yanked those changes, so they can be considered separately.

> Also, if we are bumping the format, shouldn't this format include
> Signatures? Since that is the only *functionality* missing. (The fact
> that they are slow to use is an optimization, not a functionality flaw)

I'm focused on getting bzr ready for nested trees.  It's not really a
goal to add other missing functionality.

> You also have some trailing whitespace here and there. It might be nice
> to clean that up first.

Okay.  But I don't think it matters much, unless you've configured your
editor to make it ugly.

> v- This file needs a Copyright comment and a summary doc string. (And if
> the other serializer formats don't have one, then they need them added
> as well).

Done


> +        f.write(BUNDLE_HEADER)
> +        f.write('0.9\n')
> +        f.write('#\n')
> 
> ^ it is generally better to do:
> f.write(BUNDLE_HEADER + '0.9\n#\n')
> Or at least
> f.write(BUNDLE_HEADER)
> f.write('0.9\n#\n')
> 
> Since the system call overhead is moderately big. (I realize these
> aren't writing over a Transport at least).

Well, this was in imitation of 0.8, but okay.

> v- I don't think we want to call the class StrictTestament2 but have the
> header string be 'bazaar-ng testament version 3 strict\n'
> 
> 1) We want to switch to 'bazaar' instead of 'bazaar-ng'.
> 2) If we are calling it version 3, I think it is best to call the class
> ...3 to avoid confusion.

Okay.

> +    def _escape_path(self, path):
> +        assert not contains_linebreaks(path)
> +        if path == '':
> +            path = '.'
> +        return unicode(path.replace('\\', '/').replace(' ', '\ '))
> 
> ^- Do we need replace('\\', '/') it seems that by this time, '\\' should
> not exist in the path.

I was just copying what other testament formats do.  I'd rather leave it
this way, in case it actually is doing something.

> v- this should be split across multiple lines.
> 
> -from bzrlib import inventory, treebuilder
> +from bzrlib import bzrdir, errors, inventory, repository, treebuilder

Well, I like the aesthetics of keeping it on one line until it exceeds
79 chars, but okay.

> v- You should probably update the Copyright line, but not delete it.
> (Though I'm assuming the delete was actually accidental)

Yeah.

I've attached two patches: one showing my latest changes, one showing
the difference from bzr.dev

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

iD8DBQFFGS840F+nu1YWqI0RAmc2AKCEKLlxUbjAd9DO/BK/GStH80Lw5ACeJabm
fzpXJWR8l3DH28NcKOw1juc=
=f42I
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: format-bump-full.patch
Type: text/x-patch
Size: 41182 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060926/030f1a53/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: format-bump-changes.patch
Type: text/x-patch
Size: 15467 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060926/030f1a53/attachment-0001.bin 


More information about the bazaar mailing list