Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add parameters option for hosts #243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add parameters option for hosts #243

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 7, 2020

Pull Request (PR) description

A parameters option should be added to the host resource. There are some use-cases in which this can be very helpful if you want to overwrite global parameters for a single host. (for instance the filename parameter) This implementation is very flexible.
Thank you in advance!

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

It would be nice if there was a test for this.

Also looks like #241 but with some addressed comments.

@@ -1,10 +1,12 @@
# == Define: dhcp::host
#
# @param [Hash] parameters set different parameters like e.g. filename for the host
Copy link
Member

Choose a reason for hiding this comment

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

The type is redundant and will be read from the parameter

Suggested change
# @param [Hash] parameters set different parameters like e.g. filename for the host
# @param parameters set different parameters like e.g. filename for the host

Comment on lines +24 to +25
<% @parameters.keys.sort.each do |param| -%>
<%= param %> <%= @parameters[param] %>;
Copy link
Member

Choose a reason for hiding this comment

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

Untested, but I think this also works:

Suggested change
<% @parameters.keys.sort.each do |param| -%>
<%= param %> <%= @parameters[param] %>;
<% @parameters.sort.each do |param, value| -%>
<%= param %> <%= value %>;

astritsinani pushed a commit to astritsinani/puppet-dhcp that referenced this pull request May 28, 2020
… accepting ekohl suggestion, ommited the if statement
astritsinani pushed a commit to astritsinani/puppet-dhcp that referenced this pull request May 28, 2020
… accepting ekohl suggestion, ommited the if statement
@ekohl ekohl added the needs-work not ready to merge just yet label May 10, 2022
@vox-pupuli-tasks
Copy link

Dear @ghost, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merge-conflicts needs-work not ready to merge just yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant