(This is an expansion of the discussion on this PR:
https://github.com/puppetlabs/puppet/pull/1851)
The topic of interest is the naming of 'hook' methods (e.g. 'prefetch' on a
provider, 'validate' on a type).
>From the virtual lips of Adrien Thebo:
Regarding the name, I would like us to settle on a convention for hook
methods, so that we explicitly call them out instead of having to play
"Spot the secret Puppet internal API hook methods" (which is admittedly a
very fun game, but after playing that for two years it's started to lose
some of its luster.) I like #{descriptive_name}_hook, and I got some [image:
:+1:]s in IRC, but I'm open to suggestions. Whatever is chosen, I just want
to be consistent about it. :)
My own puny opinions below:
I sort of dislike appending ‘_hook’, because:
1. It’s not consistent with other methods. I think it loses some of its
usefulness (i.e. make it easier to identify hook methods) if only a few
hook methods actually use the convention. The remedy for this seems to be
renaming all existing hook methods, but this seems like a lot of pain for a
small gain.
2. It’s only sort of helpful; it tells you to expect that something
external to the current class/module will call this method, but what, when,
and why? It’s still ultimately a magic method which you’ll need to look up
to figure out what it does.
Here are some alternative suggestions:
A. Continue with the current naming scheme (i.e. just a descriptive name,
no ‘_hook’), and focus on documenting hooks in a friendly format. For
example, it would be nice to have a clear picture of a provider’s
“interface” in the provider development docs (I googled yard docs and found
some, but they were only sort of helpful. It seems like, for example, the
'abstract' methods such as 'prefetch' are missing from the provider doc
page; not sure what's going on there). This avoids the need to rename a
bunch of stuff (or have inconsistent names), which would probably take
quite a while and be painful.
B. (more radical) Allow hook methods to be implemented as a method with any
name but require them to be “registered”, e.g.:
def my_method_name(resources)
…
end
register_hook :prefetch, :my_method_name
or maybe something more explicit (to address 2 from above) like:
Puppet::Transaction.register_hook self, :prefetch, :my_method_name
or maybe
Puppet::Transaction.prefetch_hook(self) do
…
end
This would also prevent accidentally implementing a magic method (something
that might be a concern in the PR I mentioned up top). A change like this
would probably be more trouble than it's worth, but just a thought.
I’d also be fine using Adrien’s suggestion if that’s the prevailing
opinion.
Eric
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/puppet-dev.
For more options, visit https://groups.google.com/groups/opt_out.
