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.

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

Reply via email to