> +        if resource.resource_type.is_a? Puppet::Resource::Type
> +          resource.resource_type.instantiate_resource(scope, resource)
> +        else
> +          scope.compiler.add_resource(scope, resource)
> +        end
>

I don't like this one. Can we use duck typing here instead of testing for
ancestors?

  # 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?


> -  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.

+    if ['Class', 'Node'].include? resource.type
> +      scope.catalog.tag(*resource.tags)
> +    end
>

I'd weakly-prefer this to be extracted into a method, too.

-- 
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.

Reply via email to