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

Set prerun_command with inifile instead of augeas #13

Merged
merged 1 commit into from
Oct 9, 2013
Merged

Set prerun_command with inifile instead of augeas #13

merged 1 commit into from
Oct 9, 2013

Conversation

tampakrap
Copy link
Contributor

This also fixes the deprecation warning of r10k synchronize
puppetlabs/inifile is already listed as dependency in the Modulefile (although it doesn't seem to be used anywhere so far)

@@ -38,6 +41,7 @@
$modulepath = "${r10k_basedir}/\$environment/modules"
$pe_ruby = false
}
$puppet_conf = "${puppetconf_path}/puppet.conf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

So I think $confdir is probobly a safe bet here in lieu of passing it in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't follow, do you want me to change $puppet_conf value to $confdir/puppet.conf? If so, should I also remove $puppetconf_path from that file (and replace it everywhere with $confdir)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

scratch that, I got it

@acidprime
Copy link
Collaborator

If you can refactor based on the feedback , I will get this merged , thanks for the contribution! Augeas lack of white space support annoys me too :)

@tampakrap
Copy link
Contributor Author

If you can refactor based on the feedback , I will get this merged

both comments fixed and tested

thanks for the contribution!

Thank you for this awesome module!

This also fixes the deprecation warning of `r10k synchronize`
) inherits r10k::params {

Ini_setting {
path => "$puppetconf_path/puppet.conf",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see where $puppetconf_path is being declared, I think it either needs to be added to params and redeclared as a local var like $command or perhaps just changed to $::confdir as previously discussed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is in r10k::params::puppetconf_path, lines 23 and 33 in master :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

;) I have merged but I think we should probably fully qualify the var as $r10k::params::puppetconf_path or redefine it as a param.

acidprime added a commit that referenced this pull request Oct 9, 2013
Set prerun_command with inifile instead of augeas
@acidprime acidprime merged commit 22626a2 into voxpupuli:master Oct 9, 2013
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