[MERGE] make sha1_provider a parameter to DirState()

Martin Pool mbp at canonical.com
Fri Mar 13 01:50:21 GMT 2009


Martin Pool has voted tweak.
Status is now: Conditionally approved
Comment:
So that looks pretty tidy, and I like how you're obviously just lifting 
out the relevant code in the default case.

Is this the only place the hashes come from?  I thought they could 
sometimes be passed in when building the dirstate?

I'd like to see at least one test that it actually uses a passed-in 
provider.  It's obvious by inspection here of course that you're doing 
it but it would be good to guard against future breakage.

For instance you could pass in a provider that's hardcoded to return the 
hash of the uppercase version of the file, which would be kind of 
analogous to content filtering but still let you test this in isolation. 
Then you could change the file and check that the stat and hash have 
changed.

-    def __init__(self, path):
+    def __init__(self, path, sha1_provider=None):
          """Create a  DirState object.

          :param path: The path at which the dirstate file on disk should 
live.
+        :param sha1_provider: an object meeting the Sha1Provider 
interface.
+            If None, a DefaultSha1Provider is used.


(comment) Since it's an internal interface I would have thought about 
not providing the default; we sometimes do this at multiple levels and 
it just creates cruft...

For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C49B91135.2070700%40internode.on.net%3E
Project: Bazaar



More information about the bazaar mailing list