[brisbane-core/MERGE] minor CHKMap clean-ups
John Arbash Meinel
john at arbash-meinel.com
Fri Feb 27 13:11:22 GMT 2009
-----BEGIN PGP SIGNED MESSAGE-----
Ian Clatworthy wrote:
> I did a partial review of CHKMap.py today trying to
> track down a bug (bzr ls -r-1 aren't working yet).
> Attached are some tweaks, mostly in comments, but some
> code fixes as well.
> I hope the code fixes are obvious. The only one that isn't
> is the removal of a redundant __repr__ routine - the
> superclass has identical code so it's not needed in the
> Ian C.
> PS: If we think this code is now stable enough to come into
> bzr.dev, I'll do a more formal review.
@@ -148,12 +146,13 @@
# Demand-load the root
self._root_node = self._get_node(self._root_node)
# XXX: Shouldn't this be put into _deserialize?
+ # IGC: I'm pretty sure this next line is redundant and can go
self._root_node._search_key_func = self._search_key_func
^- This is redundant with _get_node(), we can remove it, and the XXX:
@@ -200,11 +199,13 @@
for key, value in sorted(node._items.iteritems()):
+ # IGC: Shouldn't prefix and indent be used below?
result.append(' %r %r' % (key, value))
^- (This is in dump_tree, I believe)
I intentionally didn't very the indenting for keys. The reason is so
that we would have something to 'line up' on when doing
'assertEqualDiff'. So that if the overall structure changed, it would be
easier to see which bits changed, and which bits did not. For example,
If you change indenting, the whole block shows up as different, if you
leave keys at a fixed indent, then only the addition of internal
structure is changed.
I don't have an enormous preference, but that is the reason. Oh, and I
certainly don't think we want 'prefix' here, because I think it just
clutters the output.
if len(bytes) != result._current_size():
- - import pdb; pdb.set_trace()
+ #import pdb; pdb.set_trace()
raise AssertionError('_current_size computed incorrectly')
^- The pdb was intentional, but if we want to remove them, we should
strip them completely, and not just comment them out.
Otherwise it seems fine, and certainly is reasonable to land.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar