Merge lp:~ian-clatworthy/bzr/finish-commit-hook into lp:bzr/2.0

Proposed by Ian Clatworthy
Status: Rejected
Rejected by: Ian Clatworthy
Proposed branch: lp:~ian-clatworthy/bzr/finish-commit-hook
Merge into: lp:bzr/2.0
Diff against target: 81 lines
To merge this branch: bzr merge lp:~ian-clatworthy/bzr/finish-commit-hook
Reviewer Review Type Date Requested Status
John A Meinel Needs Information
Review via email: mp+10973@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

This patch adds a finish_commit hook on mutable trees mirroring the start_commit hook already there. It's required so that plugins like bzr-keywords can post-process the tree after a commit, doing things like expanding keywords with new values.

Revision history for this message
John A Meinel (jameinel) wrote :

I think I'm fine with 'finish_commit' as a hook. I'm not sure about the api it should take, or whether it should be merged for 2.0.

I would probably rather merge something like this once we have something that wants uses it, so that we know the api is reasonable. Have you already prototyped some stuff using it?

review: Needs Information
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

John A Meinel wrote:
> Review: Needs Information
> I think I'm fine with 'finish_commit' as a hook. I'm not sure about the api it should take, or whether it should be merged for 2.0.

bzr-keywords wants this so it can expand keywords in committed files,
fixing the second biggest outstanding issue with content filtering that
I know of.

> I would probably rather merge something like this once we have something that wants uses it, so that we know the api is reasonable. Have you already prototyped some stuff using it?

The proposed API is passed a MutableTree and from there, one can get the
branch and repository. That's certainly enough for most purposes I can
think of currently. If you prefer though, we can pass a more abstract
object like FinishHookParams and grab the MutableTree from an
attribute/method/property of it.

Ian C.

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> John A Meinel wrote:
>> Review: Needs Information
>> I think I'm fine with 'finish_commit' as a hook. I'm not sure about the api it should take, or whether it should be merged for 2.0.
>
> bzr-keywords wants this so it can expand keywords in committed files,
> fixing the second biggest outstanding issue with content filtering that
> I know of.
>
>> I would probably rather merge something like this once we have something that wants uses it, so that we know the api is reasonable. Have you already prototyped some stuff using it?
>
> The proposed API is passed a MutableTree and from there, one can get the
> branch and repository. That's certainly enough for most purposes I can
> think of currently. If you prefer though, we can pass a more abstract
> object like FinishHookParams and grab the MutableTree from an
> attribute/method/property of it.
>
> Ian C.
>

I do think that would be better, because then we can upgrade the api in
backwards compatible ways.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqiudIACgkQJdeBCYSNAAMX6QCgyjn7RMjuy/3BrkSk/d2EnEiP
4k8AoMJ5W1kJBsRyuim4oVE6EptTj3ue
=/uhn
-----END PGP SIGNATURE-----

Unmerged revisions

4649. By Ian Clatworthy

NEWS item

4648. By Ian Clatworthy

finish_commit hooks for mutable trees

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-08-31 00:25:33 +0000
3+++ NEWS 2009-09-01 12:35:17 +0000
4@@ -9,6 +9,13 @@
5 bzr 2.0rc2
6 ##########
7
8+New Features
9+************
10+
11+* Added finish_commit hook for mutable trees. This allows the keywords
12+ plugin to expand keywords on files changed by the commit.
13+ (Ian Clatworthy, #408841)
14+
15 Bug Fixes
16 *********
17
18
19=== modified file 'bzrlib/mutabletree.py'
20--- bzrlib/mutabletree.py 2009-07-10 08:33:11 +0000
21+++ bzrlib/mutabletree.py 2009-09-01 12:35:17 +0000
22@@ -226,6 +226,8 @@
23 revprops=revprops,
24 possible_master_transports=possible_master_transports,
25 *args, **kwargs)
26+ for hook in MutableTree.hooks['finish_commit']:
27+ hook(self)
28 return committed_id
29
30 def _gather_kinds(self, files, kinds):
31@@ -583,6 +585,10 @@
32 "hook is able to change the tree before the commit takes place. "
33 "start_commit is called with the bzrlib.tree.MutableTree that the "
34 "commit is being performed on.", (1, 4), None))
35+ self.create_hook(hooks.HookPoint('finish_commit',
36+ "Called after a commit is performed on a tree. The hook is "
37+ "called with the bzrlib.tree.MutableTree that the commit "
38+ "was performed on.", (2, 0), None))
39
40
41 # install the default hooks into the MutableTree class.
42
43=== modified file 'bzrlib/tests/per_workingtree/test_commit.py'
44--- bzrlib/tests/per_workingtree/test_commit.py 2009-08-04 11:40:59 +0000
45+++ bzrlib/tests/per_workingtree/test_commit.py 2009-09-01 12:35:17 +0000
46@@ -611,3 +611,22 @@
47 revid = tree.commit('first post')
48 committed_tree = tree.basis_tree()
49 self.assertTrue(committed_tree.has_filename("newfile"))
50+
51+ def test_finish_commit_hook(self):
52+ """Make sure a finish commit hook is called after a commit."""
53+ def finish_commit_hook_adds_file(tree):
54+ open(tree.abspath("newfile"), 'w').write("data")
55+ tree.add(["newfile"])
56+ def restoreDefaults():
57+ mutabletree.MutableTree.hooks['finish_commit'] = []
58+ self.addCleanup(restoreDefaults)
59+ tree = self.make_branch_and_tree('.')
60+ mutabletree.MutableTree.hooks.install_named_hook(
61+ 'finish_commit',
62+ finish_commit_hook_adds_file,
63+ None)
64+ self.assertFalse(tree.has_filename("newfile"))
65+ revid = tree.commit('first post')
66+ self.assertTrue(tree.has_filename("newfile"))
67+ committed_tree = tree.basis_tree()
68+ self.assertFalse(committed_tree.has_filename("newfile"))
69
70=== modified file 'bzrlib/tests/test_mutabletree.py'
71--- bzrlib/tests/test_mutabletree.py 2009-03-23 14:59:43 +0000
72+++ bzrlib/tests/test_mutabletree.py 2009-09-01 12:35:17 +0000
73@@ -30,6 +30,8 @@
74 hooks = MutableTreeHooks()
75 self.assertTrue("start_commit" in hooks,
76 "start_commit not in %s" % hooks)
77+ self.assertTrue("finish_commit" in hooks,
78+ "finish_commit not in %s" % hooks)
79
80 def test_installed_hooks_are_MutableTreeHooks(self):
81 """The installed hooks object should be a MutableTreeHooks."""

Subscribers

People subscribed via source and target branches