No subject


Sun Feb 10 21:45:11 GMT 2008


"""
Used in two places: VersionedFileStore and weaves and knits.  For weaves
and knits, when you provide this callback, when any action looks something
up, the vf will ask itself for its own scope, and if it's different for
the one that was used when it was created, it errors.  It's poked into the
VersionedFileStore because of a chicken-and-egg condition: the repo has
the thing we want to use, but it doesn't exist when the stores are
created.

This leads us to being able to remove all the ,transaction parameters.
"""

+        if None in (access_method, index):
+            raise NotImplementedError, "No default access_method or
index any more"

You should use NotImplementedError(...) syntax, and this would
probably more correctly be ValueError.

+
+def get_suffixes():
+    """Return the suffixes used by file based knits."""
+    return [DATA_SUFFIX, INDEX_SUFFIX]
+make_file_knit.get_suffixes = get_suffixes

There should be two blank lines between top level declarations.

Separately declaring a function that's later used by being bound in as a
static method that itself is sometimes treated as a class with class
variables is unusual enough that I at least wonder if it should have a
comment.

+        if self.get_scope() != self.scope:
+            raise errors.OutSideTransaction()

To the new reader this class seems to have one conceptual property,
'scope', available as either a getter method or a public member -- and yet
they are different.  Smells bad.

Also, if this is not part of the public api, why does it have public
names?

How about instead calling them say

  _scope
  _current_scope()

I realize they're only used in fairly internal classes, but I think it
still conveys something different if they have names that look like public
names.

+    def __init__(self, _format, a_bzrdir, control_files,
_revision_store, control_store, text_store):
+        super(MetaDirVersionedFileRepository, self).__init__(_format,
+                                                a_bzrdir,
+                                                control_files,
+                                                _revision_store,
+                                                control_store,
+                                                text_store)

Too long, and too widely indented.

The general shape looks ok though.

-- 
Martin



More information about the bazaar mailing list