-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Validate the values of $merge_behavior #51
Conversation
The dependency on stdlib isn't necessary. Instead of using unless $merge_behavior in ['native', 'deep', 'deeper'] {
fail("Invalid merge behavior: ${merge_behavior}")
} |
Thanks for the review, I rebased my PR |
@@ -134,6 +134,10 @@ | |||
ensure => directory, | |||
} | |||
} | |||
if $merge_behavior { | |||
unless $merge_behavior in ['native', 'deep', 'deeper'] { | |||
fail("${merge_behavior} merge behavior is invalid. Valid values are: native, deep, deeper") |
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.
this is now missing a }
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.
thanks, fixed
if $merge_behavior { | ||
unless $merge_behavior in ['native', 'deep', 'deeper'] { | ||
fail("${merge_behavior} merge behavior is invalid. Valid values are: native, deep, deeper") | ||
} |
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.
now wondering if puppet optimizes this down to:
if $merge_behavior and !($merge_behavior in ['native', 'deep', 'deeper']) {
:P
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.
logic is hard…
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.
Hah, good point. That might be a better way to specify that check.
Validate the values of $merge_behavior
This introduces dependency to puppetlabs/stdlib module