[MERGE] working tree stat cache updating during commit.

John Arbash Meinel john at arbash-meinel.com
Tue Sep 23 17:08:31 BST 2008


John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
I can understand the desire to make "_observed_sha" a private function. 
However, as it is being used outside of WT.XXX I think we have to make 
it public.
We've done this in the past, with "Repository._make_parents_provider". 
In that case Repository.get_graph(other_repo) assumes that "other_repo" 
has _make_parents_provider.

Also, I know you added a no-op version, but I think we would be 
well-served in doing double indirection for new functionality on WT, 
Branch, and Repository. So you would have:

class WorkingTree:
   def observed_sha(self, ...):
     return self._do_observed_sha()

   def _do_observed_sha(self, ...):
     # The default implementation is a no-op
     pass

I'm certainly not strict on this, but I think it would help us maintain 
compatibility in our WT/Branch/Repository apis, because we expect 3-rd 
parties (like bzr-svn) to implement portions of this. "observed_sha" 
isn't terribly likely, but I think it would be a good habit to practice.

I'll also note that your change to Builder.record_entry_contents is also 
not a backwards compatible change, and needs at least a NEWS entry. I'm 
pretty sure that bzr-svn *does* provide an implementation of Builder, so 
we'll need to give Jelmer a heads up that it is changing.
(Doing the 'do_record_entry_contents()' thing would let us update the 
record_entry_contents api, and plug in 'None' for the missing file_sha 
without requiring Jelmer to update his plugin.)

Otherwise, I'm *very* happy to see this change, and would love to see it 
merged.

For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C1222061338.9592.100.camel%40lifeless-64%3E
Project: Bazaar



More information about the bazaar mailing list