[MERGE] pre_commit hook (was: [RFC] pre_commit hook)

[John Arbash Meinel]

John Arbash Meinel has voted resubmit.
Status is now: Resubmit
Comment:
+    def capture_pre_commit_hook(self, local, master, old_revno, 
old_revid,
+                                new_revno, new_revid, affected, tree):
+        # replacing ids with paths
+        # notice that we leave deleted ids in tact
+        for change, ids in affected.iteritems():
+            if change == 'deleted':
+                continue
+            for i, id in enumerate(ids):
+                ids[i] = tree.id2path(id)
+        self.hook_calls.append(('pre_commit', old_revno, old_revid,
+                                new_revno, new_revid, affected))
+


I believe the word is 'intact' not 'in tact'.

Also, munging the ids into paths was probably less disruptive for your
tests, but means that we aren't really testing what we think we are.

I think it would probably be better to use custom ids during add, and
then check that they are mentioned.

You can do things like:

tree.add([path1, path2], [id1, id2])

Actually, I think you do that without realizing here:
+        tree.add('file', 'bang')

Is actually adding 'file' with the id 'bang'. Not adding the 2 files
'file' and 'bang'.

because of that, I think you need to redo the:
+    def test_pre_commit_paths(self):
completely. I would make it "test_pre_commit_ids()".

And I would like to see tests for adding a file, deleting a file,
renaming a file, and just modifying a file. To make sure all of those
cases show up in the 'affected_ids' list.


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3Caac8d3110708140856u67b3907fpa7909d626bf902f4%40mail.gmail.com%3E