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

Fix ignoreselected typo in protocols.conf.erb template #957

Merged
merged 2 commits into from
Sep 25, 2020

Conversation

VtG242
Copy link
Contributor

@VtG242 VtG242 commented Sep 24, 2020

Pull Request (PR) description

@igalic
Copy link
Contributor

igalic commented Sep 25, 2020

why are unit Tests failing on puppet 5 & 6?

@@ -9,8 +9,8 @@ if @values
end
end
-%>
<% if @ignore_selected != nil -%>
IgnoreSelected <%= @ignore_selected %>
<% if @ignoreselected != nil -%>
Copy link
Member

Choose a reason for hiding this comment

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

Currently, ignoreselected is a Boolean. That means it can never be nil (undef in puppet).

Copy link
Member

@alexjfisher alexjfisher Sep 25, 2020

Choose a reason for hiding this comment

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

Looks like it was always a boolean and never defaulted to undef.

@alexjfisher alexjfisher self-assigned this Sep 25, 2020
VtG242 and others added 2 commits September 25, 2020 17:52
I've also made the default for `ignoreselected` `undef`.  Up until now,
this parameter *never* worked.  By making the default `undef` (instead
of `false`), the majority of users will not see any change to their
config files and the collectd service won't get restarted.
@bastelfreak bastelfreak added the bug Something isn't working label Sep 25, 2020
@alexjfisher alexjfisher changed the title Fix of typo in variable name Fix ignoreselected typo in protocols.conf.erb template Sep 25, 2020
@alexjfisher alexjfisher merged commit f9f2e7d into voxpupuli:master Sep 25, 2020
@alexjfisher
Copy link
Member

@VtG242 Thanks for the bugfix! I've made the existing test pass by changing the default to undef and added extra tests for the cases it's explicitly set to true or false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants