Rev 4406: Respond to Andrew's review comments. in http://bazaar.launchpad.net/~jameinel/bzr/1.16-commit-fulltext

John Arbash Meinel john at arbash-meinel.com
Mon Jun 22 16:37:44 BST 2009


At http://bazaar.launchpad.net/~jameinel/bzr/1.16-commit-fulltext

------------------------------------------------------------
revno: 4406
revision-id: john at arbash-meinel.com-20090622153706-55n968lsh3v3dht7
parent: john at arbash-meinel.com-20090605140819-s7mbsn5e4ifr9xub
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 1.16-commit-fulltext
timestamp: Mon 2009-06-22 10:37:06 -0500
message:
  Respond to Andrew's review comments.
-------------- next part --------------
=== modified file 'bzrlib/groupcompress.py'
--- a/bzrlib/groupcompress.py	2009-06-05 14:08:19 +0000
+++ b/bzrlib/groupcompress.py	2009-06-22 15:37:06 +0000
@@ -1547,8 +1547,6 @@
 
         :return: An iterator over (line, key).
         """
-        if pb is None:
-            pb = progress.DummyProgress()
         keys = set(keys)
         total = len(keys)
         # we don't care about inclusions, the caller cares.
@@ -1558,13 +1556,15 @@
             'unordered', True)):
             # XXX: todo - optimise to use less than full texts.
             key = record.key
-            pb.update('Walking content', key_idx, total)
+            if pb is not None:
+                pb.update('Walking content', key_idx, total)
             if record.storage_kind == 'absent':
                 raise errors.RevisionNotPresent(key, self)
             lines = osutils.split_lines(record.get_bytes_as('fulltext'))
             for line in lines:
                 yield line, key
-        pb.update('Walking content', total, total)
+        if pb is not None:
+            pb.update('Walking content', total, total)
 
     def keys(self):
         """See VersionedFiles.keys."""
@@ -1694,7 +1694,7 @@
         if check_present:
             missing_keys = keys.difference(found_keys)
             if missing_keys:
-                raise RevisionNotPresent(missing_keys.pop(), self)
+                raise errors.RevisionNotPresent(missing_keys.pop(), self)
 
     def get_parent_map(self, keys):
         """Get a map of the parents of keys.

=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py	2009-06-04 21:09:51 +0000
+++ b/bzrlib/knit.py	2009-06-22 15:37:06 +0000
@@ -53,7 +53,7 @@
 
 
 from cStringIO import StringIO
-from itertools import izip, chain
+from itertools import izip
 import operator
 import os
 import sys
@@ -686,7 +686,7 @@
         content = knit._get_content(key)
         # adjust for the fact that serialised annotations are only key suffixes
         # for this factory.
-        if type(key) == tuple:
+        if type(key) is tuple:
             prefix = key[:-1]
             origins = content.annotate()
             result = []
@@ -935,6 +935,16 @@
         """Add a set of lines on top of version specified by parents.
 
         Any versions not present will be converted into ghosts.
+
+        :param lines: A list of strings where each one is a single line (has a
+            single newline at the end of the string) This is now optional
+            (callers can pass None). It is left in its location for backwards
+            compatibility. It should ''.join(lines) must == line_bytes
+        :param line_bytes: A single string containing the content
+
+        We pass both lines and line_bytes because different routes bring the
+        values to this function. And for memory efficiency, we don't want to
+        have to split/join on-demand.
         """
         # first thing, if the content is something we don't need to store, find
         # that out.
@@ -982,11 +992,11 @@
             lines = osutils.split_lines(line_bytes)
 
         for element in key[:-1]:
-            if type(element) != str:
+            if type(element) is not str:
                 raise TypeError("key contains non-strings: %r" % (key,))
         if key[-1] is None:
             key = key[:-1] + ('sha1:' + digest,)
-        elif type(key[-1]) != str:
+        elif type(key[-1]) is not str:
                 raise TypeError("key contains non-strings: %r" % (key,))
         # Knit hunks are still last-element only
         version_id = key[-1]
@@ -1953,7 +1963,7 @@
         chunks.extend(dense_lines or lines)
         chunks.append("end %s\n" % key[-1])
         for chunk in chunks:
-            if type(chunk) != str:
+            if type(chunk) is not str:
                 raise AssertionError(
                     'data must be plain bytes was %s' % type(chunk))
         if lines and lines[-1][-1] != '\n':
@@ -2399,7 +2409,7 @@
                     line = "\n%s %s %s %s %s :" % (
                         key[-1], ','.join(options), pos, size,
                         self._dictionary_compress(parents))
-                    if type(line) != str:
+                    if type(line) is not str:
                         raise AssertionError(
                             'data must be utf8 was %s' % type(line))
                     lines.append(line)
@@ -2594,7 +2604,7 @@
         result = set()
         # Identify all key prefixes.
         # XXX: A bit hacky, needs polish.
-        if type(self._mapper) == ConstantMapper:
+        if type(self._mapper) is ConstantMapper:
             prefixes = [()]
         else:
             relpaths = set()
@@ -2632,7 +2642,7 @@
                     del self._history
                 except NoSuchFile:
                     self._kndx_cache[prefix] = ({}, [])
-                    if type(self._mapper) == ConstantMapper:
+                    if type(self._mapper) is ConstantMapper:
                         # preserve behaviour for revisions.kndx etc.
                         self._init_index(path)
                     del self._cache
@@ -3118,7 +3128,7 @@
             opaque index memo. For _KnitKeyAccess the memo is (key, pos,
             length), where the key is the record key.
         """
-        if type(raw_data) != str:
+        if type(raw_data) is not str:
             raise AssertionError(
                 'data must be plain bytes was %s' % type(raw_data))
         result = []
@@ -3207,7 +3217,7 @@
             length), where the index field is the write_index object supplied
             to the PackAccess object.
         """
-        if type(raw_data) != str:
+        if type(raw_data) is not str:
             raise AssertionError(
                 'data must be plain bytes was %s' % type(raw_data))
         result = []

=== modified file 'bzrlib/tests/test_versionedfile.py'
--- a/bzrlib/tests/test_versionedfile.py	2009-06-04 21:09:51 +0000
+++ b/bzrlib/tests/test_versionedfile.py	2009-06-22 15:37:06 +0000
@@ -1471,18 +1471,19 @@
             self.addCleanup(lambda:self.cleanup(files))
         return files
 
+    def get_simple_key(self, suffix):
+        """Return a key for the object under test."""
+        if self.key_length == 1:
+            return (suffix,)
+        else:
+            return ('FileA',) + (suffix,)
+
     def test_add_lines(self):
         f = self.get_versionedfiles()
-        if self.key_length == 1:
-            key0 = ('r0',)
-            key1 = ('r1',)
-            key2 = ('r2',)
-            keyf = ('foo',)
-        else:
-            key0 = ('fid', 'r0')
-            key1 = ('fid', 'r1')
-            key2 = ('fid', 'r2')
-            keyf = ('fid', 'foo')
+        key0 = self.get_simple_key('r0')
+        key1 = self.get_simple_key('r1')
+        key2 = self.get_simple_key('r2')
+        keyf = self.get_simple_key('foo')
         f.add_lines(key0, [], ['a\n', 'b\n'])
         if self.graph:
             f.add_lines(key1, [key0], ['b\n', 'c\n'])
@@ -1499,16 +1500,10 @@
 
     def test__add_text(self):
         f = self.get_versionedfiles()
-        if self.key_length == 1:
-            key0 = ('r0',)
-            key1 = ('r1',)
-            key2 = ('r2',)
-            keyf = ('foo',)
-        else:
-            key0 = ('fid', 'r0')
-            key1 = ('fid', 'r1')
-            key2 = ('fid', 'r2')
-            keyf = ('fid', 'foo')
+        key0 = self.get_simple_key('r0')
+        key1 = self.get_simple_key('r1')
+        key2 = self.get_simple_key('r2')
+        keyf = self.get_simple_key('foo')
         f._add_text(key0, [], 'a\nb\n')
         if self.graph:
             f._add_text(key1, [key0], 'b\nc\n')
@@ -1758,13 +1753,6 @@
         self.capture_stream(files, entries, seen.add, parent_map)
         self.assertEqual(set(keys), seen)
 
-    def get_simple_key(self, suffix):
-        """Return a key for the object under test."""
-        if self.key_length == 1:
-            return (suffix,)
-        else:
-            return ('FileA',) + (suffix,)
-
     def get_keys_and_sort_order(self):
         """Get diamond test keys list, and their sort ordering."""
         if self.key_length == 1:



More information about the bazaar-commits mailing list