If the relay parameter is not set, the empty? errors out.#449
If the relay parameter is not set, the empty? errors out.#449deligatedgeek wants to merge 1 commit intovoxpupuli:masterfrom
Conversation
towo
left a comment
towo
left a comment
There was a problem hiding this comment.
LGTM, seeing how we're doing the same check in the next line anyway.
|
Description could've used a bit more love. :) |
Sorry, thought I had put in a better description. No idea where I pasted that 8-) |
smortex
left a comment
smortex
left a comment
There was a problem hiding this comment.
Sorry, I started to write a review but realized I had not finished it and it was still pending.
I feel like better / more complete unit test in this area would have helped spot this issue, but it is another story.
| case resource[:backend].to_s | ||
| when 'relay' | ||
| t << "olcRelay: #{resource[:relay]}\n" unless resource[:relay].empty? | ||
| t << "olcRelay: #{resource[:relay]}\n" if resource[:relay] |
There was a problem hiding this comment.
An empty string/array now evaluates to a truthy value, so this will change the code behavior.
If we can also pass undef it may make sense to check resource[:relay].nil? and resource[:relay].empty?:
| t << "olcRelay: #{resource[:relay]}\n" if resource[:relay] | |
| t << "olcRelay: #{resource[:relay]}\n" unless resource[:relay].nil? || resource[:relay].empty? |
You can describe the wrong behavior you fix in the PR first message. It will be linked to in the generated ChangeLog with the next release of the module. |
3 participants
Pull Request (PR) description
This Pull Request (PR) fixes the following issues