-
Notifications
You must be signed in to change notification settings - Fork 448
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.
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
linters: allow "categories" for subfiltering #4077
linters: allow "categories" for subfiltering #4077
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #4077 +/- ##
==========================================
- Coverage 94.60% 94.60% -0.01%
==========================================
Files 641 641
Lines 51842 51822 -20
==========================================
- Hits 49045 49026 -19
+ Misses 2797 2796 -1
... and 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
0d86b74
to
f9b9ac8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, nice and clean.
Can we extend the ros related spread test that uses the extension to verify this is working as expected too?
Additionally, can we have a spread test that declares linter entries?
Do you mean by e.g. checking the
Yes. I'll do both in a little bit. |
@sergiusens thank you for the suggestion, it pointed out an unexpected interaction. First, I added a new spread test that uses the Then I tried to add a new spread with a "user-provided" lint ignore like: lint:
ignore:
- unused-library:
- usr/lib/*/libpng16.so* ... the resulting behavior is that this new What do you think? |
2c8cc94
to
729462a
Compare
This commit adds the notion of "categories" for Linter subclasses. For linters that perform multiple different "types" of linting, this lets the author ignore specific paths for specific types of linting issues, with a finer granularity than ignoring the whole linter wholesale. Currently the only Linter type that supports this is the LibraryLinter, which provides the "unused-library" and "missing-library" categories to disable checking for unused or missing libraries.
This lets us ignore the many (40+) "unused library" warnings that the linter emits for the libraries in opt/ros/ and related.
729462a
to
4ac70be
Compare
@tigarmo let's ask that question during the demo |
This commit adds the notion of "categories" for Linter subclasses. For linters that perform multiple different "types" of linting, this lets the author ignore specific paths for specific types of linting issues, with a finer granularity than ignoring the whole linter wholesale.
Currently the only Linter type that supports this is the LibraryLinter, which provides the "unused-library" and "missing-library" categories to disable checking for unused or missing libraries.
CRAFT-1642
make lint
?pytest tests/unit
?First commit has the general linter changes, second commit has the update of the ros extension.