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

Update ComparisonToEmptyStringRule.py #770

Merged
merged 2 commits into from
May 8, 2020
Merged

Update ComparisonToEmptyStringRule.py #770

merged 2 commits into from
May 8, 2020

Conversation

vbotka
Copy link
Contributor

@vbotka vbotka commented May 7, 2020

The conditionals with bare variables will be deprecated. It's causing a lint warning atm. See CONDITIONAL_BARE_VARS https://docs.ansible.com/ansible/latest/reference_appendices/config.html#conditional-bare-vars

The conditionals with bare variables will be deprecated. It's causing a lint warning atm. See CONDITIONAL_BARE_VARS https://docs.ansible.com/ansible/latest/reference_appendices/config.html#conditional-bare-vars
Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

fair enough

@webknjaz
Copy link
Member

webknjaz commented May 7, 2020

While the change is ok, I cannot accept it like this because you should also update the docs so that the CI doesn't fail: https://github.com/ansible/ansible-lint/pull/770/checks?check_run_id=652398319#step:10:53.

@vbotka vbotka mentioned this pull request May 7, 2020
@vbotka
Copy link
Contributor Author

vbotka commented May 7, 2020

... update the docs so that the CI doesn't fail: https://github.com/ansible/ansible-lint/pull/770/checks?check_run_id=652398319#step:10:53.

Thank you for the review. I've updated the docs and submitted PR #772

Will you be able to squash and merge #770 and #772 ?

@webknjaz
Copy link
Member

webknjaz commented May 7, 2020

Just update this PR, it'll only be green if they are together.

@ssbarnea
Copy link
Member

ssbarnea commented May 8, 2020

@vbotka Please close the doc update and include it here. This can be avoided by always running tox -e lint before any push.

@vbotka
Copy link
Contributor Author

vbotka commented May 8, 2020

Done. "tox -e lint" passed before the push. Thank you for comments.

@webknjaz webknjaz merged commit 6fb857a into ansible:master May 8, 2020
@vbotka vbotka deleted the patch-1 branch May 8, 2020 16:12
@webknjaz
Copy link
Member

webknjaz commented May 8, 2020

Looks green to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants