-
Notifications
You must be signed in to change notification settings - Fork 356
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: conditional statements jinja2 warning #685
Conversation
When ansible-core version is updated from 2.15.4 to 2.15.8 the following warning occurs: [WARNING]: conditional statements should not include jinja2 templating delimiters such as {{ }} or {% %} Fixed by removing the delimeters. Signed-off-by: Ilari Iso-Junno <[email protected]>
tasks/validate/validate.yml
Outdated
if ansible_facts['distribution'] | lower in ['alpine', 'ubuntu'] else ansible_facts['distribution_major_version'] in nginx_distributions[ansible_facts['distribution'] | lower]['versions'] | string }}" | ||
- "{{ ansible_facts['architecture'] in nginx_distributions[ansible_facts['distribution'] | lower]['architectures'] }}" | ||
- ansible_facts['distribution'] | lower in nginx_distributions.keys() | list | ||
- (ansible_facts['distribution_version'] | regex_search('\\d{1,2}\\.\\d{2}') | float in nginx_distributions[ansible_facts['distribution'] | lower]['versions'] | map('float') |
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.
Can you move the parentheses back to where they were? I've run into situations using the "X if Y" conditional in Ansible has led to issues when the "X" was not wrapped in parentheses.
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.
See the comment I made in the review 😄
Based on my manual testing, the short parenthesis (code before if) did not
add any value and therefore removed them.
The two line block did not work without parenthesis around the whole block.
Therefore added them.
Feel free to add the short parenthesis back. I can do it next Tuesday.
pe 12. tammik. 2024 klo 22.53 Alessandro Fael Garcia <
***@***.***> kirjoitti:
… ***@***.**** requested changes on this pull request.
See the comment I made in the review 😄
—
Reply to this email directly, view it on GitHub
<#685 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACIWNXSAZCIRCPKS7IADI6DYOGPFLAVCNFSM6AAAAABBYLHNAOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMJZGAZDQNBSGE>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
It's not needed in this case, but it's there for consistency around this role and the config role. That being said, after removing the curly braces and double quotes, something (e.g. parentheses) had to be used to ensure that it works given that it's a multiline conditional. I'll readd them on Monday 😄 |
I ended up flattening everything into a single line 👌 |
When ansible-core version is updated from 2.15.4 to 2.15.8 the following warning occurs:
[WARNING]: conditional statements should not include jinja2 templating delimiters such as {{ }} or {% %}
Fixed by removing the delimeters.
Proposed changes
Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue using one of the supported keywords here in this description (not in the title of the PR).
Checklist
Before creating a PR, run through this checklist and mark each as complete:
CONTRIBUTING
document.defaults/main/*.yml
,README.md
andCHANGELOG.md
).