[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