Fixed pre-commit hooks #612
Conversation
Assures that when used as a pre-commit hook, ansible-lint is not given each file as argument. This is needed because ansible-lint needs to detect the code-base layout in order to figure-out which YAML files are playbooks, tasks or vars files. Previously the hook was passing all files that where matching its filters and ansible-lint would assume they were playbooks. This made it give false positives. This change also adds the --force-color because without it ansible-lint would fail to detect the TTY and avoid using colors. Fixes: #611 Signed-off-by: Sorin Sbarnea <ssbarnea@redhat.com>
|
Those wanting to test it before it merges can run this: This would call pre-commit hook from this branch. |
|
Hm... Not sure if that's what I want — it breaks the assumption that pre-commit would only lint changed bits.
It'd be interesting to add this right to the CI. I see it implemented as |
|
@webknjaz I don't think that ansible-lint is the kind of linter that can be used on files without context. What if you try to install newer version? The newer version could find bugs in files that were not touched. And yes, the idea about adding to CI is great! |
|
That command for CI doesn't work as I expected. It probably needs to be reported to pre-commit. |
| @@ -6,6 +6,9 @@ | |||
| - id: ansible-lint | |||
| name: Ansible-lint | |||
| description: This hook runs ansible-lint. | |||
| entry: ansible-lint | |||
| entry: ansible-lint --force-color . | |||
There was a problem hiding this comment.
--force-color shouldn't be necessary as of pre-commit 1.19
There was a problem hiding this comment.
in fact, it's the wrong thing to do if you're using a CI system, etc. which doesn't have color escapes
| # do not pass files to ansible-lint, see: | ||
| # https://github.com/ansible/ansible-lint/issues/611 | ||
| pass_filenames: false | ||
| always_run: true |
There was a problem hiding this comment.
this ~usually isn't the setting you want here -- this'll do a lot of unnecessary work on every commit (linting your entire codebase all the time) -- generally you want to optimize for quickness on commit and being thorough in CI (via --all-files, etc.)
you'd want |
Assures that when used as a pre-commit hook, ansible-lint is not
given each file as argument. This is needed because ansible-lint
needs to detect the code-base layout in order to figure-out which
YAML files are playbooks, tasks or vars files.
Previously the hook was passing all files that where matching its
filters and ansible-lint would assume they were playbooks. This made it
give false positives.
This change also adds the --force-color because without it ansible-lint
would fail to detect the TTY and avoid using colors.
Fixes: #611
Signed-off-by: Sorin Sbarnea ssbarnea@redhat.com