[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.repo._revision_store.all_revision_ids(transaction)])
+ 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
VersionedFiles...
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.
--
Martin
More information about the bazaar
mailing list