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

Replace bool filter with 'is truthy' #12

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

toby-griffiths
Copy link
Sponsor Contributor

Replaces the ' | bool' filters with ' is truthy'.

I was havving issues with this always being false, even when I set the mas_email or mas_password values. Having debugged the mas_email var to ensure it was making it in to the role correctly I raised an issue on ansible, and was advised this is the correct way to check for non-empty strings.

Replaces the ' | bool' filters with ' is truthy'.
@toby-griffiths
Copy link
Sponsor Contributor Author

Currently the mas_email isn't tested in the tests. Would you like me to add the variables to the tests (although it's broken as discussed in #11)?

@geerlingguy
Copy link
Owner

This could be a little funky, because (a) that test (truthy) was only added in Ansible 2.10, meaning users with Ansible 2.9 would not be able to use this collection—and that test does very slightly change the potential behavior of this check (it doesn't return an actual boolean value unless configured that way, afaict reading the docs).

@toby-griffiths
Copy link
Sponsor Contributor Author

toby-griffiths commented Jun 29, 2021

Hmmm… I see your point. Would conditional tasks, depending on ansible version work?

Odd that the've changed the functionality of the | bool filter, or is there something I've missed?

@geerlingguy
Copy link
Owner

I'm thinking it might be worth breaking the 2.9 compatibility... people should be running the latest version anyways (IMO) and 2.10's been out long enough (and 3.0/4.0) that any new users should be able to get it.

@geerlingguy geerlingguy merged commit ca66c07 into geerlingguy:master Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants