Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Fix lint issue #170

Merged
merged 2 commits into from
Sep 2, 2022
Merged

Conversation

zhan9san
Copy link
Member

Related to #163

@zhan9san zhan9san added the bug This issue/PR relates to a bug. label Aug 28, 2022
loop: "{{ molecule_yml.platforms }}"
register: dockerfile_stats

- name: Create Dockerfiles from image names
ansible.builtin.template:
# when using embedded playbooks the dockerfile is alonside them
src: >-
{%- if dockerfile_stats.results[i].stat.exists -%}
Copy link
Member

Choose a reason for hiding this comment

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

Are you really sure that removing the "-" does not break the template? AFAIK the linter does not touch these and altering them has high risk of damaging the generate template.

Copy link
Member Author

@zhan9san zhan9san Aug 28, 2022

Choose a reason for hiding this comment

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

I see. Sorry I forgot to mark it as a draft PR.
It's lucky we have test to stop me doing this.

Currently, there is an extra space between before else.

Actual: }} {%- else -%}
Expected by linter: }}{%- else -%}

Copy link
Member Author

@zhan9san zhan9san Aug 28, 2022

Choose a reason for hiding this comment

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

It seems only if we put all statement in one single line, the linter can pass.
{%- if foo -%}...{%- endif -%}

But it will make the line too long.

@ssbarnea
Do you have any suggestion?
https://github.com/ansible-community/molecule-docker/runs/8033866754?check_suite_focus=true#step:9:122

Copy link
Member Author

@zhan9san zhan9san Aug 28, 2022

Choose a reason for hiding this comment

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

Do you still have any concern about this PR?

Folding without spaces

Using double quote and back slash is the only way supporting folding without spaces.

Using a “Folded Block Scalar” > will fold newlines to spaces;

But ansible-lint is not happy with the spaces.

BTW, I tried to remove if-else statement in Jinja, but use set_fact module with Conditionals.
As it's a for-loop, it's not elegant either.

Test with ansible-lint==6.5.0

❯ ansible-lint --version                               
ansible-lint 6.5.0 using ansible 2.13.3
~/src/github/molecule-docker feature/fix-lint-issue*                                                                                                      23:50:00
❯ ansible-lint src/molecule_docker/playbooks/create.yml
~/src/github/molecule-docker feature/fix-lint-issue*                                                                                                      23:50:11
❯ echo $?                                                                                                    
0

Reference

https://docs.ansible.com/ansible/2.9/reference_appendices/YAMLSyntax.html
https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-in-yaml-over-multiple-lines/21699210#21699210

Copy link
Member Author

Choose a reason for hiding this comment

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

@ssbarnea

Sorry to bother you.

Could you kindly review this PR?

@zhan9san zhan9san marked this pull request as draft August 28, 2022 14:06
@zhan9san zhan9san marked this pull request as ready for review August 28, 2022 15:37
@ssbarnea ssbarnea merged commit 2e844e0 into ansible-community:main Sep 2, 2022
@zhan9san zhan9san deleted the feature/fix-lint-issue branch September 3, 2022 01:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants