Skip to content
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

Improve jinja spacing #413

Merged
merged 2 commits into from
Jul 14, 2023
Merged

Improve jinja spacing #413

merged 2 commits into from
Jul 14, 2023

Conversation

pa-decarvalho
Copy link
Contributor

This pull request aims to improve jinja spacing. To achieve this, the jinja[spacing] section has been removed from the .config/ansible-lint.yml file.

While making the correction, I came across the following variable | bool and decided to remove all | bool occurrences from the project as they are not necessary. Feel free to let me know if I should create another PR for this part or revert the change.

remove the jinja[spacing] section from the ansible-lint configuration and address lint errors
@vitabaks
Copy link
Owner

@pa-decarvalho

First and foremost, thank you for your contribution to the project. Your efforts towards improving jinja spacing are greatly appreciated, and your attention to detail is commendable.

However, I'd like to bring up the topic of removing all | bool occurrences. The use of the | bool filter in Ansible is considered a good practice. It ensures consistent boolean interpretation across various possible representations, such as "True", "true", "yes", "on", "1", etc. The | bool filter ensures these values are interpreted in their boolean representation, facilitating comparisons and usage in conditionals among others.

If you've encountered a situation where | bool is misused or not needed, that might indeed be a case for correction. But removing all occurrences of | bool without appropriate analysis and evaluation might lead to unforeseen consequences.

Looking forward to your thoughts on this.

cc @ThomasSanson

@pa-decarvalho
Copy link
Contributor Author

@vitabaks

I didn't know that | bool was a good practice. Usually, when I work on an Ansible project, all the linters are active, and values other than true and false are not allowed for booleans (like yes, 1...)

I will immediately replace all occurrences of | bool to avoid any unpleasant surprises.

@vitabaks
Copy link
Owner

@ThomasSanson What do you think about bool filter?

@pa-decarvalho
Copy link
Contributor Author

@ThomasSanson What do you think about bool filter?

@vitabaks @ThomasSanson

I am in the process of reverting back regarding the bool filter and correcting only lint errors (I am waiting for the tests to pass locally).

If you agree, I can create an issue for us to discuss the pros and cons of using it.

@vitabaks
Copy link
Owner

vitabaks commented Jul 13, 2023

thread for discussion of the bool filter #415

Copy link
Sponsor Contributor

@ThomasSanson ThomasSanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitabaks

I have completed the task of reviewing all 28 files.

I can confirm that everything is in order from my perspective.

Great job, @pa-decarvalho, and thank you! 🎉🙏

Now, let's move on to the next task! 💪😊

@vitabaks vitabaks merged commit 51f17d6 into vitabaks:master Jul 14, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants