[MERGE] updated index bisection
Martin Pool
mbp at canonical.com
Thu Oct 11 10:47:40 BST 2007
Martin Pool has voted comment.
Status is now: Semi-approved
Comment:
I think you need a little bit of commentary at the Index level
explaining the concepts which are added:
key references
that the index can be partially parsed into memory, and
we remember both what keys we've seen and what bytes they were
self._nodes = None
Since you're adding comments about the others it would be good to add
one for
_nodes, particularly since its function is less clear if the index is
partially parsed.
+ def _resolve_references(self, references):
+ """Return the resolved key references for references."""
The docstring doesn't tell me much more than the method name...
+ def lookup_keys_via_location(self, location_keys):
+ """Public interface for implementing bisection.
Is this intended to be a public interface? It looks more like a special
callback for bisection and so it shouldn't be public.
This method is really a bit long. It looks like it should be possible
to split up into its several sub-tasks of searching the cache, getting
the headers,
+ :param location_keys: A list of location, key tuples.
+ :return: A list of (location_key, result) tuples as expected by
+ bzrlib.bisect_multi.bisect_multi_bytes.
And location is a byte offset?
You have some cases of missing spaces around = for assignment.
It looks like the index has three almost disjoint states and behaviors:
entirely loaded, partially loaded, and not loaded at all. I wonder if
it would be simpler to factor out an inner object with two classes for
the different strategies.
s/ajust/adjust
to be continued...
For details, see:
http://bundlebuggy.aaronbentley.com/request/%3C1192078676.15845.1.camel%40lifeless-64%3E
More information about the bazaar
mailing list