----- Original Message -----
> On Mon, May 16, 2011 at 14:20, R.I.Pienaar <[email protected]> wrote:
> > ----- Original Message -----
> >> On Mon, May 16, 2011 at 14:02, R.I.Pienaar <[email protected]> wrote:
> >> > Allow . in fact names
> >>
> >> This doesn't seem sufficient to me: facter, at least, defines the
> >> fact name as "any arbitrary string, after String#downcase", which
> >> means
> >> that this is still substantially more restrictive than one of the
> >> underlying libraries it models.
> >
> > supporting that full facter set wont work - for example facter
> > supports
> > spaces in fact names which makes no sense.
> >
> > I'd suggest facter is allowing way more than it should - so rather
> > than
> > introduce a whole bunch of new bugs by complying to what facter
> > allows
> > I think it best to just limit to these.
>
> It would be great if you could file that bug against Facter, and call
> that out in the commit message then. That way when we go back
> looking
> at the history here it will be clear what went on. In context of
> which branch, etc, I agree that this is probably the right changes.
Well in this specific case - a . character - this just isnt something
Facter should support since puppet cant have dots in vars so this case
is an odd case that facter/puppet would never support. And fwiw these
kind of tweaks are pretty common in the code base, the DDL validations
are usually a bit tight and I loosen them up as I get reports back so
it's a fairly common thing.
For mc use these never get turned into actual variables so I can allow
a few more than puppet but certainly want to keep that kind of thing to
a minimum. So in this case a '.' is a namespace separator where:
{"foo" => {"bar" => 1}}
becomes
"foo.bar" => 1
I'll file a bug though that facter should just support the variables
allowable in puppet vars.
>
> (...and I totally agree that the definition of valid names in Facter
> is *cough* weak.)
>
> >> It also lacks tests, and the commit message that is too short to
> >> hit
> >> our coding standards. Those need to be cleaned up before this can
> >> go
> >> into the code.
> >
> > A lot of the code lacks tests - tests are being retrofitted - so
> > there's
> > no test harness at all for the DDL files right now and makign this
> > testable
> > will mean refactors to the code that just wont be allowed on the
> > prod bug-fix
> > only release branch so this would not be possible.
>
> Yeah, that makes sense. Now I understand why that was done this way,
> and I agree entirely.
>
> Daniel
> --
> ⎋ Puppet Labs Developer – http://puppetlabs.com
> ✉ Daniel Pittman <[email protected]>
> ✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
> ♲ Made with 100 percent post-consumer electrons
>
> --
> 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.
>
>
--
R.I.Pienaar
--
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.
