ghoneycutt / puppet-module-facter

Puppet module to manage Facter
Other
2 stars 22 forks source link

Add option to specify custom fact to be in file #11

Closed kentjohansson closed 9 years ago

kentjohansson commented 10 years ago

To be able to push facts out to the clients

Phil-Friderici commented 10 years ago

I would love to see this functionality be available in upstream :)

ghoneycutt commented 10 years ago

@kentjohansson This needs to be rebased to be ready for merge. What do you think about not having the match param and just setting match to ^${fact_name}=

kentjohansson commented 10 years ago

I’ve change the default value for $match. So you don't need to specify it to get the expected behaviour. Or you think that we should remove the option to change it all together?

Rebased as well

ghoneycutt commented 10 years ago

Looking good!

We should have a test where you specify that facter::fact::file is /etc/facter/facts.d/facts.txt and show that everything compiles correctly without error.

kentjohansson commented 10 years ago

Changed to "^${name}=\S*$"

Why would you specify the default value? It's not covered by https://github.com/kentjohansson/puppet-module-facter/blob/file_custom_facts/spec/classes/init_spec.rb#L190

ghoneycutt commented 10 years ago

To capture the logic you placed on line 14 of fact.pp

kentjohansson commented 10 years ago

Like this? https://github.com/kentjohansson/puppet-module-facter/blob/file_custom_facts/spec/classes/init_spec.rb#L236

ghoneycutt commented 10 years ago

There should be a spec test for the define itself, such as this one https://github.com/ghoneycutt/puppet-module-types/blob/master/spec/defines/mount_spec.rb

In that, specify that $file is /etc/facter/facts.d/facts.txt and show that everything compiles correctly without error.

kentjohansson commented 10 years ago

Added spec/defines/fact_spec.rb (I think that is what you meant) Changed init_spec.rb back as it was before. Squashed commits

kentjohansson commented 10 years ago

Added extra should contain_file_line in spec/defines/fact_spec.rb

kentjohansson commented 10 years ago

Double quoted match string

ghost commented 10 years ago

@ghoneycutt @kentjohansson Has there been any progress on this pull request?

kentjohansson commented 10 years ago

Been so long, so new rebase and squash Could you have a new look @ghoneycutt ?

Phil-Friderici commented 10 years ago

@kentjohansson thank you for rebasing this :)

ghost commented 10 years ago

I have been running this code in production for a while (rebased locally). It has worked fine so far.

ghoneycutt commented 9 years ago

Awesome work! I made a few changes and will be merging from #19