Skip to content
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

(MODULES-5953) Adds ability to set stringify_facts #259

Conversation

petems
Copy link
Contributor

@petems petems commented Nov 8, 2017

  • stringify_facts is currently not configurable
    in the module
  • Lets add a class to configure it
  • AND add docs on using the puppet_conf task to
    Do it

}

} else {
fail('The puppet_agent::prepare::stringify_facts class should only be run on Puppet < 4')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is slightly awkward, because it means you have to apply this class, then remove it and apply puppet_agent to upgrade. Letting it be a warning might be slightly friendlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought the stringify_facts setting would throw an error with Puppet 4, but it's just ignored, so I'll switch this to a warning 👍

@puppetcla
Copy link

CLA signed by all contributors.

* `stringily_facts` is currently not configurable 
in the module
* Lets add a class to configure it
* AND add docs on using the `puppet_conf` task to
Do it
@petems petems force-pushed the MODULES-5953-add_stringify_facts_class_and_docs branch from 8fc3dfe to 760eca1 Compare November 9, 2017 15:04
@petems
Copy link
Contributor Author

petems commented Nov 9, 2017

@MikaelSmith Fixed to a warning 👍

@MikaelSmith
Copy link
Contributor

MikaelSmith commented Nov 9, 2017

@branan care if we include this in the next release? We could hold off while we do a bugfix release, then plan to add it in a later feature release.

But it only applies to people starting on Puppet 3, so I'd rather get it out sooner if we're going to do it at all.

@MikaelSmith MikaelSmith merged commit 74a2910 into puppetlabs:master Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants