-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support fact stringyfication #908
Conversation
Tested at voxpupuli/puppet-allknowingdns#64 |
moduleroot/spec/spec_helper.rb.erb
Outdated
@@ -20,6 +21,12 @@ RSpec.configure do |c| | |||
c.hiera_config = <%= @configs['hiera_config'] %> | |||
end | |||
<%- end -%> | |||
<%- if @configs['facterdb_string_keys'] -%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd tempted to always write it out to become independent of the default in the gem. That way we can just flip the default without breaking things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's why I added a default to config_defaults.yml. But I can remove the if condition here if you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but the default is false
. If you want it unconditionally, you should check for .nil?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if I can follow your thoughts. My idea was to always add the facterdb_string_keys
option with a default of false
and individual modules can switch to true
. And if we ever want to, we can set the default to true
in rspec-puppet-facts
and that works with the current code. Did you have something else in mind?
366a5fb
to
870fcd3
Compare
verified again at https://github.com/voxpupuli/puppet-allknowingdns/pull/64/commits |
FacterDB/rspec-puppet-facts have an option switch the symbolized first-level keys in a factset to strings (all following/nested keys are always strings). This PR makes this configuration opt-in so we can switch over modules from symbols to strings in their spec files.
FacterDB/rspec-puppet-facts have an option switch the symbolized first-level keys in a factset to strings (all following/nested keys are always strings). This PR makes this configuration opt-in so we can switch over modules from symbols to strings in their spec files.