Fix for allow_bash_command_substitution #30
Conversation
Fixes bug where string is being passed where Boolean is expected
| @@ -121,7 +121,7 @@ dont_blame_nrpe=<%= $dont_blame_nrpe %> | |||
|
|
|||
| #allow_bash_command_substitution=0 | |||
| <% if $allow_bash_command_substitution { -%> | |||
There was a problem hiding this comment.
It looks like the original intention was to default to whatever nrpe's default is (possibly version dependent?) by not writing anything but allowing either true/false to explicitly set the option.
There was a problem hiding this comment.
Not sure if it's important to some people to be able to explicitly set allow_bash_command_substitution=0 (even if that has always been the default??). Either way, I've opened #32 with just a fix to the template and the tests I should have added when I broke everything 16 months ago! :)
|
Dear @jorhett, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
|
Dear @jorhett, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
|
Dear @jorhett, thanks for the PR! This is Vox Pupuli Tasks, your friendly Vox Pupuli Github Bot. I noticed that your pull request has CI failures. Can you please have a look at the failing CI jobs? |
Pull Request (PR) description
Allow
allow_bash_command_substitutionto be set.This Pull Request (PR) fixes the following issues
config manifest sets the value to a string of 0 or 1, but the template expects a Boolean. Since we need to allow for undef value, best to keep it Boolean all the way through.