[MERGE] add lock hooks

Martin Pool mbp at canonical.com
Tue Apr 8 03:18:11 BST 2008


On Tue, Apr 8, 2008 at 6:50 AM, Robert Collins
<robertc at robertcollins.net> wrote:
>  > +class PhysicalLockHooks(Hooks):
>  > +    """hooks for physical lock activity."""
>  >
>  > s//Hooks/
>
>  Do you mean 'Use a capital letter?'

yes

>
>
>  > Shouldn't this be a private class?
>
>  bzrlib.branch.BranchHooks isn't named with an _ either; one of the
>  benefits of the current hook layout is that these classes are public :P.
>
>
>  > +
>  > +# The hooks instance clients should register against. Do *NOT*
>  > import
>  > this
>  > +# symbol, always reference it through lock.hooks.
>  > +hooks = PhysicalLockHooks()
>  >
>  > It's unfortunate the current standard is an API that can so easily be
>  > used incorrectly.  (But, again, not in scope for this particular
>  > patch.)
>
>  Its harder to use incorrectly in other modules because there is a top
>  level class like Branch that it can be an attribute of.

Since these are LockDir-specific hooks maybe they should be there?
Otoh in the future we want them to be more general.

Maybe we should introduce a base class Lock at least to hold this?

-- 
Martin <http://launchpad.net/~mbp/>



More information about the bazaar mailing list