-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 deprecation warning about performing a regex comparison on a hash #2293
Fix deprecation warning about performing a regex comparison on a hash #2293
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you have the same problem on line 24?
Hmm, good question. Here's line 24: <%- if !(_item['require'] && _item['require'] != '') && _item['require'] !~ /unmanaged/i && !(_item['auth_require']) -%>When |
|
@ekohl Are you good with the above explanation?? ^^^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right and the whole _item['require'] !~ /unmanaged/i is redundant. It's complex.
If I'm reading it right, this is equal:
<%- if (!_item['require'] || _item['require'] == '') && _item['require'] !~ /unmanaged/i && !(_item['auth_require']) -%>
Now if you read that form, it must be either undef or ''. Both of those are never /unmanaged/i so it's redundant, thus equal to
<%- if (!_item['require'] || _item['require'] == '') && !(_item['auth_require']) -%>
Once you read that, it can actually become an elsif for the block above so it becomes:
<%- if _item['auth_require'] -%>
Require <%= _item['auth_require'] %>
<%- elsif !_item['require'] || _item['require'] == '' -%>
Require all granted
<%- end -%>If my logic there is correct, I think we don't need to modify it now.
Just to clarify: if Submitted my suggestion as a separate PR: #2303 |
|
Ok, everything looks good to merge then |
I'm getting warnings like this for each
puppet agentrun:This PR changes the logic to only performing the regex comparison when the
requiresvalue is a string.