> Fix is_virtual fact to return strings rather than bools.
>
> Wouldn't a better way to achieve this be this patch?
>
> diff --git a/lib/facter/util/fact.rb b/lib/facter/util/fact.rb
> index e78ed97..e01833b 100644
> --- a/lib/facter/util/fact.rb
> +++ b/lib/facter/util/fact.rb
> @@ -77,6 +77,9 @@ class Facter::Util::Fact
>                 break tmp unless tmp.nil? or tmp == ""
>             }
>
> +            # Fact values must necessarily be strings, enforce that.
> +            @value = @value.to_s
> +
>             unless foundsuits
>                 Facter.debug "Found no suitable resolves of %s for %s" %
> [[email protected], @name]
>             end
>

Yes but...

1) I'd still suggest my suggested simplification on the original code (but
your approach makes it off topic).
2)

Otherwise the uninitiated might make the sad and painful discovery of this
> themselves by returning false and having to learn that only '' is actually
> "not true" as far as puppet is concerned.
>

If I'm reading this correctly neither patch will work, as both result in
"false" rather than "".

3) Looking at the surrounding (gawd-awful) code I'm not convinced this is
exactly the right spot for it...but that inject-with-next-unless-and-break
pattern makes my head hurt.  It looks as if this change would sometimes stop
the "value for %s is still nil" branch from firing.   Maybe it should be
done right before the "return @value"?

-- M
-----------------------------------------------------------
The power of accurate observation is
commonly called cynicism by those
who have not got it.  ~George Bernard Shaw
------------------------------------------------------------

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