On Tue, Oct 12, 2010 at 12:46 PM, Jesse A Wolfe <[email protected]> wrote:
> # Make an instance of our resource type. This is only possible >> # for those classes and nodes that don't have any arguments, and is >> # only useful for things like the 'include' function. >> > > The bit about "don't have any arguments" seems stale, doesn't it? > You're right. I will fix this. > > >> - def mk_plain_resource(scope) >> + def mk_singleton_resource(scope) >> > > I agree that "mk_plain_resource" is not a good name, but I'm not sure if > "singleton resource" is much clearer - is that a new coinage or are we > already using it as jargon elsewhere? (I recognize that we've got some > naming work to do outside the scope of this patch, we need a more > descriptive word for ["classes" "nodes" and "defined types"] than > "Resource::Type") > > and if you're going to be renaming it anyway, let's lose the "mk". I think > it stands for "make", but the abbreviation is unnecessary, and "make" is a > nearly meaningless word in this context. > > The two pieces of information I want to be captured (or at least hinted at) in the name are the fact at (a) the method both creates the resource and installs it in the catalog--it's not just an accessor, and (b) the method is idempotent--it doesn't do anything if the resource has already been created and installed in the catalog. (a) is what motivated keeping "mk" in the name, and (b) is what motivated the word "singleton". How would you feel about calling it "ensure_in_catalog"? > + if ['Class', 'Node'].include? resource.type >> + scope.catalog.tag(*resource.tags) >> + end > > > I'd weakly-prefer this to be extracted into a method, too. > Ok, I don't see any harm in that. I'll fold that change in. -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
