[MERGE] add a hook for Branch.set_last_revision_info()

Aaron Bentley aaron at aaronbentley.com
Wed Apr 9 05:01:25 BST 2008


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

James Henstridge wrote:
> On IRC Aaron suggested that the branch being changed be exposed as a
> member of the params object passed to the hook rather than as a
> separate argument.  The attached bundle makes that change.

Thanks.

bb:resubmit

> +    def _make_branch_tip_hook_params(self, new_revno, new_revision_id):
> +        """Construct a Params object for passing to *branch_tip hooks."""
> +        if Branch.hooks['post_change_branch_tip']:
> +            old_revno, old_revision_id = self.last_revision_info()
> +            return ChangeBranchTipParams(
> +                self, old_revno, new_revno, old_revision_id, new_revision_id)
> +        else:
> +            return None

For myself, I'd prefer to invoke _make_branch_tip_hook_params after
updating to the new revno and revision_id, since they're only
*predicted* values at this point.  But that's probably just anal.  You
don't have to make this change; it's just a thought.

> @@ -1395,10 +1447,12 @@
>          configured to check constraints on history, in which case this may not
>          be permitted.
>          """
> +        hook_params = self._make_branch_tip_hook_params(revno, revision_id)
>          revision_id = _mod_revision.ensure_null(revision_id)

^^ _make_branch_tip_hook_params should not be invoked before
ensure_null.  ensure_null makes sure the revision_id is a string, not None.

> @@ -1853,10 +1907,12 @@
>  
>      @needs_write_lock
>      def set_last_revision_info(self, revno, revision_id):
> +        hook_params = self._make_branch_tip_hook_params(revno, revision_id)
>          revision_id = _mod_revision.ensure_null(revision_id)

^^^ wrong order again.

> +post_change_branch_tip
> +----------------------
> +
> +Run after a branch tip has been changed but while the branch is still
> +write-locked. Note that push, pull, commit and uncommit all invoke this hook.
> +
> +The hook signature is (params), where params is an object containing
> +the members
> +
> +  branch
> +    The branch whose tip has been changed.
> +
> +  old_revno
> +    The revision number (eg 10) of the branch before the change.

^^^ Maybe mention that this is always an integer, never a dotted revno?

> +class TestPostChangeBranchTip(TestCaseWithMemoryTransport):
> +
> +    def setUp(self):
> +        self.hook_calls = []
> +        TestCaseWithMemoryTransport.setUp(self)
> +
> +    def capture_post_change_branch_tip_hook(self, params):
> +        """Capture post_change_branch_tip hook calls to self.hook_calls.
> +
> +        The call is logged, as is some state of the branch.
> +        """
> +        self.hook_calls.append((params, params.branch.is_locked()))
> +
> +    def test_post_change_branch_tip_empty_history(self):
> +        branch = self.make_branch('source')
> +        Branch.hooks.install_hook('post_change_branch_tip',
> +                                  self.capture_post_change_branch_tip_hook)

It looks like this hook is never being removed.  But if not, how are the
remaining tests passing?

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH/D+V0F+nu1YWqI0RAmy4AJ4oeRiwMAvJWvCw/QmvyZij59nahACdFbJA
I6kATKJBSraYBcSoaKTF8JU=
=exHd
-----END PGP SIGNATURE-----



More information about the bazaar mailing list