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.

Sign up for GitHub

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

Jump to bottom

Ensure that yamllint config ignore entries are used #1346

Merged
merged 4 commits into from Feb 14, 2021
Merged

Ensure that yamllint config ignore entries are used #1346

merged 4 commits into from Feb 14, 2021

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Feb 14, 2021 •
edited by ssbarnea

There are two problems why ignore entries were ignored:

  1. The way YamlLintConfig.extend() works with ignore entries, it is important to use the extended config instead of the original one.
  2. The path needs to be supplied to run() so that yamllint can process its ignore config entries.

Fixes: #1344

@felixfontein felixfontein mentioned this pull request Feb 14, 2021
Closed
@felixfontein
Copy link
Contributor Author

felixfontein commented Feb 14, 2021 •
edited

Well, the CI failure is an interesting one: the .yamllint file in the repo's root explicitly ignores one of the test files, and if ansible-lint uses the .yamllint ignore entries, this causes the test to not work correctly since ansible-lint no longer reports the yamllint problems in that file.

@ssbarnea ssbarnea self-requested a review February 14, 2021 08:51
@ssbarnea ssbarnea added the bug label Feb 14, 2021
@felixfontein
Change into examples/ for test.
922ed99 
This ensures that the top-level .yamllint config does not prevent some of the
errors to be reported in this file.
@felixfontein
Copy link
Contributor Author

@ssbarnea finally the tests passed ;)

@ssbarnea ssbarnea merged commit 4a1b96c into ansible:master Feb 14, 2021
@felixfontein felixfontein deleted the use-yamllint-config-properly branch February 14, 2021 09:40
@felixfontein
Copy link
Contributor Author

@ssbarnea thanks for reviewing and merging!

@geerlingguy
Copy link

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssbarnea ssbarnea ssbarnea approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

.yamllint file in root directory is not used when ansible-lint runs yamllint
3 participants
@felixfontein @geerlingguy @ssbarnea