[MERGE] Tree._iter_changes

[John Arbash Meinel]

Aaron Bentley wrote:
> Hi all,
> 
> This bundle implements the Tree._iter_changes API, which is intended to
> unify our various change-detection routines.
> 

I'm replying here because BB seems unhappy with me. But +1 conditional.

Sorry I didn't get to this yesterday.

I'm not positive that making the interface private is sufficient,
considering it is being actively used by parts of the code. But I'm
willing to pretend it is, so that we can move forward.

v- To be consistent, the parameter should probably be named 'stat_value'.
I realize it involves changing a little more code, but it is better to
have a nicer api.
-    def _fingerprint(self, abspath):
-        try:
-            fs = os.lstat(abspath)
-        except OSError:
-            # might be missing, etc
-            return None
+    def _fingerprint(self, abspath, fs=None):
+        if fs is None:
+            try:
+                fs = os.lstat(abspath)
+            except OSError:
+                # might be missing, etc
+                return None


Why did you chose to iterate over to and then from, rather than
iterating over both simultaneously (as I did in _compare_trees())?

One thing I'd really like to see as I move forward with splitting up
inventory, or at least adding a recursive 'last_changed' indicator, is
that we don't have to iterate over the complete inventories in the
common case. Anyway, it isn't critical. It is nice to have the
information in a single location, since it makes it easier to fix
everything by just fixing one place.

So my only real request is to use 'stat_value' instead of 'fs'.
Otherwise +1. The rest is just for discussion.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061201/33616f5a/attachment.pgp