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-4264) Update for puppet 4 data types #437

Merged
merged 4 commits into from
Apr 11, 2017

Conversation

hunner
Copy link
Contributor

@hunner hunner commented Mar 27, 2017

The data types for validation are using the puppet 4 types with
validate_legacy to check for any legacy types that will be removed on
the next release. This should be a backwards-compatible change except
that it uses puppet 4's data types.

@hunner
Copy link
Contributor Author

hunner commented Mar 27, 2017

Actually I should get this on top of #424. Oops.

Variant[String, Stdlib::Compat::String] $target,
$ensure = undef,
Optional[Variant[String, Stdlib::Compat::String]] $content = undef,
Optional[Variant[String, Stdlib::Compat::String, Array]] $source = undef,
Copy link
Contributor

Choose a reason for hiding this comment

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

Add Stdlib::Compat::Array to the variant also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

) {
validate_string($target)
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of straight up removing these, use the validate_legacy function here: https://github.com/puppetlabs/puppetlabs-stdlib/blob/master/lib/puppet/functions/validate_legacy.rb
A description of why/usage: https://github.com/puppetlabs/puppetlabs-stdlib#validate_legacy

Copy link
Contributor

@HelenCampbell HelenCampbell left a comment

Choose a reason for hiding this comment

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

Looks awesome! Just a couple of places you left things out, then it looks good to go 👍

@hunner hunner force-pushed the puppet4 branch 4 times, most recently from b0698be to b560943 Compare March 31, 2017 19:47
metadata.json Outdated
@@ -103,7 +103,7 @@
"requirements": [
{
"name": "puppet",
"version_requirement": ">= 3.0.0 < 5.0.0"
"version_requirement": ">= 4.5.2 < 5.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be 4.6.1? As far as I know this is the first puppet4 version that is considered stable. Vox Pupuli bumped everything to 4.6.1. All based on this statement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Preferably 4.6.1 because yes it works better (mostly around create_resources bugs like https://tickets.puppetlabs.com/browse/PUP-6230), but the 4.5 series was the first that fixed data-in-modules (https://tickets.puppetlabs.com/browse/PUP-6230). I'll check with the gang.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PE 2016.4 (LTS) includes 4.7 but the previous release 2016.2 (they skipped .3) includes 4.5.2... but according to https://docs.puppet.com/pe/latest/overview_getting_support.html 2016.2 is EOL April 30, 2017 so we could probably just jump straight to 4.7.0 support-wise, and use the older puppet-3 versions of modules for supporting 2016.2 and earlier until they EOL.

The data types for validation are using the puppet 4 types with
validate_legacy to check for any legacy types that will be removed on
the next release. This should be a backwards-compatible release except
that it uses puppet 4's data types and bumps metadata.
@tphoney
Copy link
Contributor

tphoney commented Apr 11, 2017

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants