[MERGE] Hook documentation

Martin Pool mbp at canonical.com
Wed Oct 29 05:57:04 GMT 2008


2008/10/20 Michael Ernst <mernst at alum.mit.edu>:
> I have incorporated them all in the attached merge directive, except for
> one:
>
>> > The first parameter ``'post_push'``
>>
>> I'm not sure that the markup of �1r|``'post_push'``�1r} makes sense �1rt why include
>> single quotes here, inside the monospaced text?
>
> This formatting is retained from before my patch.  I presume it's because
> that's the actual parameter is a single-quoted 9-character string, and the
> text is referring to the actual argument.
>
>    branch.Branch.hooks.install_named_hook('post_push', post_push_hook,
>                                     'My post_push hook')
>
> (Actually, "argument" may be a better term than "parameter", if Python
> doesn't have some different convention for the terms.  A standard
> convention is "actual argument", "formal parameter".)

Yes, I believe 'argument' would make it more clear that it's a value,
not the formal parameter name.

> -The class of each hook is given immediately after each hook type below.
> +The class of each hook is given in parentheses immediately after each hook
> +type below.
> +

Seeing this in your diff I realize what's already there is not quite
clear - it's not so much the class of each hook, but the class that
contains the hook.

> +Each description also indicates whether the hook runs on the client (the
> +machine with the local branch) or the server (the machine with the remote
> +branch).  These may be, but are not necessarily, the same machine.

I saw spiv's comments on this in the previous review.  How about

  the client (the machine where you invoked bzr) or the server (the
machine addressed
  by the branch or repository URL.)

It's not pedant-proof but it may be closer or clearer.


More information about the bazaar mailing list