[MERGE] add lock hooks
Robert Collins
robertc at robertcollins.net
Mon Apr 7 21:50:34 BST 2008
On Fri, 2008-04-04 at 00:33 -0400, Martin Pool wrote:
>
> The name and the docstring implies these are active for all physical
> locks, but they are only implemented for LockDirs, and we do
> currently
> also use OS locks. I think the hook name is probably best kept
> generic,
> but you should at minimum document this in the dev guide, as well as
> in
> news.
>
> It's unfortunate there's no namespace allowing you to reach all hooks.
>
> +class PhysicalLockHooks(Hooks):
> + """hooks for physical lock activity."""
>
> s//Hooks/
Do you mean 'Use a capital letter?'
> 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.
> +
> +class LockResult(object):
>
> Docstring?
> +
> + def record_hook(self, result):
> + self._calls.append(result)
> +
> + def reset_hooks(self):
> + self._old_hooks = lock.hooks
> + self.addCleanup(self.restore_hooks)
> + lock.hooks = lock.PhysicalLockHooks()
> +
>
> How about putting these on a separate class and then you can factor
> the common setup into the setUp method?
Hmm, I need other logic from the current class too though; I think I can
put the self._calls = [] line in reset_hooks though.
-Rob
--
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080408/e5973005/attachment.pgp
More information about the bazaar
mailing list