Add rule violations from Yamllint #955
Conversation
ssbarnea
added
the
feedback-needed
ssbarnea
force-pushed
the
feature/yamllint
branch
from
53f4555
to
9f0ff5a
Compare
ssbarnea
force-pushed
the
feature/yamllint
branch
2 times, most recently
from
68fe7fd
to
2bc5071
Compare
ssbarnea
added
major
ssbarnea
requested review from
greg-hellings,
albinvass,
LukeShortCloud,
felixfontein and
odyssey4me
|
I've tried this PR in a role-only collection of mine, and found it ignores my .yamllint file. |
ssbarnea
force-pushed
the
feature/yamllint
branch
from
2bc5071
to
636fec1
Compare
|
@felixfontein Thanks for looking at it. Yep, I am aware that it ignores the in-repo yamllint config and that it uses a hardcoded configuration. That is more or less by design as I wanted to fully control the results. I can see how this could get me into trouble and I guess I will have to make it use the embedded default only when a local config is not found. One thing that you likely already know is that most Ansible content is not compatible with the yamllint-own-default-config and that the author has very strong opinions regarding what defaults should be. Another feature that is currently missing is ability to fully disable this rule instead of ignoring or skipping it. I do not expect to merge this very soon but getting early feedback is essential, as it would allow me to improve the implementation. |
ssbarnea
force-pushed
the
feature/yamllint
branch
from
636fec1
to
9666597
Compare
|
@ssbarnea being able to disable that would be really helpful :) Does it make sense to make this configurable? I don't know if ansible-lint has other rules that can be configured (and not just disabled). |
ssbarnea
force-pushed
the
feature/yamllint
branch
2 times, most recently
from
754d29e
to
d91c316
Compare
ssbarnea
force-pushed
the
feature/yamllint
branch
from
d91c316
to
05019fe
Compare
Replace use of class with an instance of a Rule for MatchErrors, as this will allows us to process rules that produce various different error messages (aka meta-rules). Prepares-For: #955
Replace use of class with an instance of a Rule for MatchErrors, as this will allows us to process rules that produce various different error messages (aka meta-rules). Prepares-For: #955
ssbarnea
force-pushed
the
feature/yamllint
branch
3 times, most recently
from
082b510
to
3287ddd
Compare
Now is configurable, it will respect user defined config. You can also skip entire rules with "YAML" tag, or specific ones with their named-tag reported by yamllint itself. AFAIK, it is ready to be merged. I am just waiting for an approval. Once merged I will tag a pre-release in order to allow users to start consuming the newer changes. |
Also detect yamllint rule violations when this is found as installed. This will allow us to remove a set of rules that overlapped with general YAML syntax ones. Fixes: #953
ssbarnea
force-pushed
the
feature/yamllint
branch
from
3287ddd
to
1bc3008
Compare
|
@ssbarnea thanks for implementing this! |
|
@felixfontein Thanks for reviewing it. I also have https://github.com/ansible-community/ansible-lint/pull/950/files ready for review, which is kinda similar: making use of ansible syntax check. |
| DESCRIPTION = """\ | ||
| Rule violations reported by YamlLint when this is installed. | ||
|
|
||
| You can fully disable all of them by adding 'YAML' to the 'skip_list'. |
There was a problem hiding this comment.
Just to be sure: This is supposed to with YAML in the skip_list, not with yaml? To me it seems like the other way around?
There was a problem hiding this comment.
That is an error, correct key is 'yaml', I will fix it.
Yaml not specific to Ansible will use a different linter as while ansible-lint does detect yamllint violations (see ansible/ansible-lint#955), I would prefer that yaml not specific to Ansible be linted in a more generic sense.
This detect yamllint rule violations when this is found as installed. This will allow us to remove a set of rules that
overlapped with general YAML syntax ones.
Fixes: #953
Release-Notes: #1150