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

Added globaloptions parameter to dhcp class #41

Merged
merged 1 commit into from
Mar 30, 2015

Conversation

dgmorales
Copy link
Contributor

This allows one to add arbitrary global options to the main dhcpd.conf file, the same way the module already deals with arbitrary options in a specific pool.

@puppetcla
Copy link

Waiting for CLA signature by @dgmorales

@dgmorales - We require a Contributor License Agreement (CLA) for people who contribute to Puppet, but we have an easy click-through license with instructions, which is available at https://cla.puppetlabs.com/

Note: if your contribution is trivial and you think it may be exempt from the CLA, please post a short reply to this comment with details. http://docs.puppetlabs.com/community/trivial_patch_exemption.html

@dgmorales
Copy link
Contributor Author

It is trivial, and I guess it fits your definition of trivial too, but I signed it anyway. Maybe I'll make larger contributions in the future.

@underscorgan
Copy link
Contributor

@dgmorales thanks for the contribution, and apologies it took so long to get back to you. Do you think you could update spec/classes/dhcp_spec.rb with tests for the new parameter?

@tphoney
Copy link
Contributor

tphoney commented Mar 2, 2015

@dgmorales ping? have you had a chance to look at the tests ?

@dgmorales
Copy link
Contributor Author

@tphoney Hello, sorry for my silence. I'll take a look at it in a few days.

I thought I would be able to do it way sooner, so I didn't answer before. I don't have experience with spec tests yet, but this is a nice opportunity to start changing that.

@tphoney
Copy link
Contributor

tphoney commented Mar 2, 2015

@dgmorales Thanks for the effort you are putting in, there are loads of examples in other modules and http://ask.puppetlabs.com/questions/ is a great resource too.

@dgmorales
Copy link
Contributor Author

I've updated this PR with the tests. I've also rebased it to the current master so it applies cleanly. Hope that's OK.

Good learning. In the process I've discovered that dhcp_conf_header was being set to 'eth0', making my tests fail (actually any test based on the content of dhcpd.conf would fail).

I've also tried to refactor spec/classes/dhcp_spec.rb so to set a default param of interface => 'eth0' once in the top, instead of setting it in each context. But that got me to more strange errors that I couldn't understand yet.

@@ -19,6 +19,7 @@
$default_lease_time = 3600,
$max_lease_time = 86400,
$service_ensure = running,
$globaloptions = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you keep the trailing comma here?

@underscorgan
Copy link
Contributor

@dgmorales thanks, I had one more comment, and otherwise if you could squash this down to a single commit it should be ready to go!

@dgmorales
Copy link
Contributor Author

@mhaskel done, take a look.

underscorgan pushed a commit that referenced this pull request Mar 30, 2015
Added globaloptions parameter to dhcp class
@underscorgan underscorgan merged commit 692d65f into voxpupuli:master Mar 30, 2015
@underscorgan
Copy link
Contributor

Looks good, thanks @dgmorales !

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.

4 participants