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

Add hiera_hash support for limits fragments #106

Merged
merged 1 commit into from
Apr 2, 2015

Conversation

jwennerberg
Copy link

No description provided.

@@ -837,6 +838,19 @@
validate_re($sshd_pam_access, $valid_pam_access_values,
"pam::sshd_pam_access is <${sshd_pam_access}> and must be either 'required', 'requisite', 'sufficient', 'optional' or 'absent'.")

case type($limits_fragments_hiera_merge) {
Copy link
Owner

Choose a reason for hiding this comment

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

The type() function is deprecated as type is now a reserved word. Check out the is_* functions.

https://github.com/puppetlabs/puppetlabs-stdlib/tree/4.2.0#is_string

@ghoneycutt
Copy link
Owner

Great work @jwennerberg Glad to see some code from you again :)

@jwennerberg
Copy link
Author

about time I got back to some coding :)

Let me know what you think and if/when to rebase.

@jwennerberg
Copy link
Author

@ghoneycutt did you have a chance to look at this?

if is_bool($limits_fragments_hiera_merge) == true {
$limits_fragments_hiera_merge_real = $limits_fragments_hiera_merge
} else {
validate_re($limits_fragments_hiera_merge, '^(true|false)$', "pam::limits_fragments_hiera_merge may be either 'true' or 'false' and is set to <${limits_fragments_hiera_merge}>.")
Copy link
Owner

Choose a reason for hiding this comment

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

don't think this is needed since str2bool will sort it out

@jwennerberg
Copy link
Author

you mean something like that?

@@ -837,6 +838,9 @@
validate_re($sshd_pam_access, $valid_pam_access_values,
"pam::sshd_pam_access is <${sshd_pam_access}> and must be either 'required', 'requisite', 'sufficient', 'optional' or 'absent'.")

$limits_fragments_hiera_merge_real = str2bool($limits_fragments_hiera_merge)
Copy link
Owner

Choose a reason for hiding this comment

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

I think you need a conditional around this in case the variable is already a boolean.

something like

if is_string($limits_fragments_hiera_merge) == true {
  $limits_fragments_hiera_merge_real = str2bool($limits_fragments_hiera_merge)
} else {
  $limits_fragments_hiera_merge_real = $limits_fragments_hiera_merge
}
validate_bool($limits_fragments_hiera_merge_real)

@jwennerberg jwennerberg force-pushed the hiera_hash branch 3 times, most recently from 788117c to 6a88d7c Compare April 1, 2015 15:57
@jwennerberg
Copy link
Author

@ghoneycutt I've rebased this against master

ghoneycutt added a commit that referenced this pull request Apr 2, 2015
Add hiera_hash support for limits fragments
@ghoneycutt ghoneycutt merged commit fc56dbc into ghoneycutt:master Apr 2, 2015
@ghoneycutt
Copy link
Owner

Released in v2.17.0

Thanks @jwennerberg

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.

2 participants