Closed iisojunn closed 10 months ago
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 https://github.com/nginxinc/ansible-role-nginx/pull/685#pullrequestreview-1819028421, 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
).