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
Flake8 output file support. #10371
Flake8 output file support. #10371
Conversation
8d1eac7
to
bd8b697
Compare
Working on adding tests, but otherwise this is ready. Any feedback @Eric-Arellano @stuhood ? |
test added. this is g2g. |
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.
Thanks! Just wondering if we can generalize this at all.
register( | ||
"--output-file", | ||
type=str, | ||
metavar="filename", | ||
default=None, | ||
advanced=True, | ||
help="Redirect report to a file.", | ||
) |
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.
Rather than an output file, could this potentially be an output directory, located on the Lint
Goal
? That way each linter could emit its output in subdirectories of a shared output directory... and we could probably give it a reasonable default so that reports are always generated.
Since the LintOutputFile
type is exposed to the Goal
, it would be good to enable more linters to be able to use it (without specifying an output file per linter).
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.
ok, updated.
But keep in mind that now we hard code the report file name for each linter in the rule.
Which is not optimal, since each linter has other options to control the format of the file (using pass-thru args or by adding plugins to the requirements).
so the issue there is that the file name will not reflect the format of the file.
We can obviously expose the file name as an option on each linter, but then, a user will have to configure things in two places, which is confusing.
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.
True... but it feels like whether or not a linter has multiple file formats is a per linter concern... the linter can choose which filename it wants to within the directory. Fine either way.
Similar to #10371 ### Problem CI Systems (among other things) can usually pick up output of linters and display them in a more use friendly way (similar idea to junitxml for tests) Currently, there is no way to have bandit write the violations it detects to a file (passthru args won't work due to pants engine isolation requirements) ### Solution Add explicit support for the bandit --output paramter and extract the output and write it to the requested directory. [ci skip-rust]
Problem
CI Systems (among other things) can usually pick up output of linters and display them in a more use friendly way (similar idea to junitxml for tests)
Currently, there is no way to have flake8 write the violations it detects to a file (passthru args won't work due to pants engine isolation requirements)
Solution
Add explicit support for the flake8
--output-file
paramter and extract the output and write it to the requested directory.I added this on the top level linter since other linters (pylint & bandit) also have similar capabilities, so we will be able to add similar features to those rules down the line.