On 10/20/2010 12:36 AM, Markus Roberts wrote:
Angelos --
I poked at it a bit but didn't have any blinding insights. A few
observations:
> # Verify that the passed value is valid.
# If the developer uses a 'validate' hook, this method will get
overridden.
def unsafe_validate(value)
self.class.value_collection.validate(value)
+ validate_features_per_value(value)
end
As the comment notes, putting the call to validate_features_per_value
here is at risk because it could be trounced by the validate hook. It
may make more sense to put it in validate, right after the call to
unsafe_validate (but still inside the begin block) for this reason.
That said, I don't think that's causing the problem you are seeing.
Yah, I wasn't sure if we want to allow someone overriding validate to
skip the required_features validation. But it makes sense that we'd want
to enforce that.
@@ -299,16 +285,6 @@ class Puppet::Property < Puppet::Parameter
# If the developer uses a 'validate' hook, this method will get
overridden.
def unsafe_validate(value)
super
- validate_features_per_value(value)
- end
You can just eliminate the whole method if all it does is call super.
Thanks, already done locally.
Overall it looks pretty good, but I suspect I'm missing something.
What are you using for a test case? Can you update the ticket with a
bit more information about how you're trying it, and maybe that will
suggest something.
Hrm. Now I came in for work and tried it all again, it works (i.e. it
produces a warning) as expected. I guess it was a PEBKAC error, probably
missed an install or server restart and have been chasing ghosts since.
So I'll attach my patch to the bug report and will mail puppetlabs the
signed CLA later today.
Thanks for your help,
Aggelos
--
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.
