[MERGE] Fix non-rich to rich-root fetch

Aaron Bentley aaron at aaronbentley.com
Tue Apr 29 16:15:06 BST 2008


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

Ian Clatworthy wrote:
>>          inventory_weave = self.source.get_inventory_weave()
>> -        parent_texts = {}
>>          versionedfile = {}
> 
> inventory_weave isn't used anywhere in this routine so can go?
> versionedfile (as a map) isn't either now.

Gone.

> I think this block of code needs to be a method in its own right,
> returning planned_revisions and revision_root.

Okay.  It seems like an arbitrary division to me, but done.

> I didn't spent much time thinking though the performance
> implications of the new code. The multiple calls to iter_rev_trees()
> stands out though.

iter_rev_trees is being called with completely different revisions as
its input, so the performance impact should be small.

> This looks ok. I'd appreciate it though if the comment also explained
> that parents get dropped if their revision_root doesn't match, and why
> you're doing that.

Okay, I've done so.  It seemed obvious to me that files cannot have
different files as their parents.

> * make_tree_and_repo() made the first method in the class
> * do_test and the two routines using it put right next to each other
>   (without get_parents in the middle)

Done.

> do_test() is probably just a little *too* generic a name as well given
> the numerous other tests covered by the class. do_fetch_order_test() or
> do_order_test() would be better I feel.

Done.

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

iD8DBQFIFzt60F+nu1YWqI0RAkIqAJ9LSFoXAcvjAS/jCh/kh4WZCOTpmQCdENz3
W4wApVa1VpB0Lc6H5MHF+Rw=
=JNjB
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fetch-non-rich-to-rich.patch
Type: text/x-diff
Size: 16502 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080429/806da996/attachment.bin 


More information about the bazaar mailing list