[MERGE] Update to bzr.dev.

Martin Pool mbp at canonical.com
Tue Jun 17 08:31:39 BST 2008

I wish you had more of a cover letter for some of these large changes.  It
would make some of the review faster either through helping people
understand what's going on, or letting them question it at a conceptual
rather than line-by-line level.

I know you have talked quite a lot either on the list or one-to-one about
what you're doing but it'd be good to have it altogether in a final form
as this comes in.

Maybe you could start a text file as the branch goes along and add to it
as you work through it.

=== modified file 'bzrlib/multiparent.py'
+def topo_iter_keys(vf, keys=None):
+    if keys is None:
+        keys = vf.keys()
+    parents = vf.get_parent_map(keys)
+    return _topo_iter(parents, keys)

A docstring would be nice.  All your callers pass a keys parameter so I
don't think it should be optional or that None should be accepted.  APIs
that iterate the whole thing are generally something we'd consider evil.
I realize that was in the previous API topo_iter but now's a good chance
to get rid of it.

=== modified file 'bzrlib/reconcile.py'
@@ -213,9 +214,10 @@
         from bzrlib.weave import WeaveFile, Weave
         transaction = self.repo.get_transaction()
         self.pb.update('Reading inventory data.')
-        self.inventory = self.repo.get_inventory_weave()
+        self.inventory = self.repo.inventories
+        self.revisions = self.repo.revisions
         # the total set of revisions to process
-        self.pending = set([rev_id for rev_id in
+        self.pending = set([key[-1] for key in self.revisions.keys()])

         # mapping from revision_id to parents
         self._rev_graph = {}

Hm, I realize it's consistent with the existing code but having
a member called 'inventory' that is not actually an inventory is not so
great.  Come to that it is maybe confusing that .revisions are not
(directly) revisions.

+        versions = [key[-1] for key in self.revisions.keys()]

Can't help thinking that this should be done as
self.revisions.all_revision_ids() or something, specific to that

Other than that, no other comments on reconcile.py.  The diff is pretty
big and while not quite mechanical I can see it's mostly keeping quite
similar logic, and I didn't spot anything that was not mapped across.

=== modified file 'bzrlib/repofmt/pack_repo.py'

+        data_access = _DirectPackAccess(
+                {self.new_pack.text_index:self.new_pack.access_tuple()})

I believe pep8 calls for a space after the colon in dict literals.

No other comments on that file.


More information about the bazaar mailing list