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

John Arbash Meinel john at arbash-meinel.com
Tue Aug 14 17:11:19 BST 2007

John Arbash Meinel has voted resubmit.
Status is now: Resubmit
+    def capture_pre_commit_hook(self, local, master, old_revno, 
+                                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: 

More information about the bazaar mailing list