Puppet » Facter

• Do we need a new fact for ‘windowsrelease’? Can you tell me what value this might add?

I don’t have a strong reason, but its seems that using xp or 7 is easier than remembering kernel versions. Although, If I get some help we might be able to make it a regex instead of comparing to constants. I got stuck on the how to account for “2008 R2”

• If there is value in windowsrelease … creating a new mswindows.rb util is unnecessary. Its only used for getting the release. Why don’t you just do that work in your ‘windowsrelease’ fact, and re-use the fact information in operatingsystem.rb?

will do

• can you take a look at the recent changes to kernelrelease.rb and see if you can use the way we now do WMI queries as well?

yes

Other then that – once we get a version can you rebase and submit a pull request via git on that branch please? Sorry to be a hassle – its just easier for us :–). And since you’re doing it in github anyway I presume its easier for you as well :–).

yes

I’m going to mark this ‘Code Insufficient’ for now – but don’t get the wrong idea … the code is pretty good its just the organisation I’m commenting on here :–). And since we recently did a Windows sprint some items have changed a tad.

Thanks Ken! Also I might be around puppetlabs on friday if you want to chat.

So my comment about ‘windowsrelease’ just be clear was more around me being dumb and wondering why it is separate from ‘operatingsystemrelease’ in your patch. What do you get from windowsrelease? Do you have sample output?

I’d love to catch up and chat – but I’m a London employee which means I’m normally far far away :–).

Ken Barber wrote:

So my comment about ‘windowsrelease’ just be clear was more around me being dumb and wondering why it is separate from ‘operatingsystemrelease’ in your patch. What do you get from windowsrelease? Do you have sample output?

oh windowsrelease is the caption returned by wmi and operatingsystemrelease is just the “7” from that caption.

If I finish the tests for #7643 and it gets merged then windowsrelease would be duplicated because it also returns caption. Maybe I should remove it from this commit.

C:\Users\blkperl>WMIC OS GET CAPTION

Microsoft Windows 7 Professional

C:\Users\blkperl>WMIC OS GET CAPTION

Microsoft Windows Server 2008 R2 Enterprise

If I finish the tests for #7643 and it gets merged then windowsrelease would be duplicated because it also returns caption. Maybe I should remove it from this commit.

Leaving it out and have it displayed through your other WMI generic facility sounds like a more sound approach to me. I’m just conscientious about namespace clutter :–).