search

adrienverge / yamllint

A linter for YAML files.
GNU General Public License v3.0
2.85k stars 273 forks source link

skip ignored files at discovery #646

Open james-powis opened 8 months ago

james-powis commented 8 months ago

Fixes: https://github.com/adrienverge/yamllint/issues/399

Not sure how coverage decreased, I did not touch that file ;)

ignored files should be skipped early on to prevent other issues such as tripping on intentionally ignored broken symlinks.

There are many reasons that a file is ignored, one of those reasons is that it is a symlink which for valid reasons is broken. Without this change, this results in an [Errno 2] No such file or directory: error which can cause CI/CD to fail.

There likely is efficiencies by skipping to read in the skipped files at startup, but that is not the point of this PR.

coveralls commented 8 months ago

Coverage Status

coverage: 99.797% (-0.03%) from 99.822% when pulling 5417f9619c03c060b8e689bcb951b9a023650d74 on james-powis:skip-ignored-files-on-load into 0d7ae7bf6b852fd739ba385b686ea83ed3164a79 on adrienverge:master.

adrienverge commented 8 months ago

Thanks for contributing!

Not sure how coverage decreased, I did not touch that file ;)

Coverage decreased indeed, because changes to yamllint source code should come with tests, to make sure there won't be regression. It was requested in the original issue #399 ;) A new test simulating the scenario of the initial message of #399 should be fine. See tests/test_config.py for how existing tests look like.

If you add a check not conf.is_file_ignored(filepath) early, should this check be removed later in yamllint execution? (I'm not 100% sure, but I remember that touching find_files_recursively() + conf.is_file_ignored() is tricky and could create problems, it needs special attention)

james-powis commented 8 months ago

@adrienverge I did not touch linter.py so its coverage decreasing is not related to my change to cli.py and likely a related to something else....

Furthermore there is a pretty significant gordian knot related to these checks inside and out within the code. Refactoring your code is not my interest. I was just trying to contribute a fix, which this does, and your users have been complaining about for over 2 years, which I also was impacted by. I have a monkey patch in place and do not need you to accept this. I just figured I would try and help out. This is an exceptionally trivial change, and one which is exceptionally difficult to write a test for. Its just not that valuable for me to deal with that silly difficulty. However I am sure you could piece together the test modifications in my other PR if you need inspiration.