Run python linter in CI #5238
Run python linter in CI #5238
Conversation
|
@pmwkaa, @mkindahl: please review this pull request.
|
Codecov Report
@@ Coverage Diff @@
## main #5238 +/- ##
===========================================
+ Coverage 63.03% 89.04% +26.00%
===========================================
Files 225 225
Lines 45706 51853 +6147
===========================================
+ Hits 28812 46171 +17359
+ Misses 16894 5682 -11212
Continue to review full report at Codecov.
|
|
Same as mentioned in another PR: #5237 (comment) |
| on: | ||
| "on": |
There was a problem hiding this comment.
Not how it is done in other YAML files.
There was a problem hiding this comment.
Yeah, that's for the next PR: #5237
Apparently on is a truthy value, and somehow this applies to keys as well, so it's better to quote it. Same as the famous Norway problem :)
There was a problem hiding this comment.
No strong opinion on doing one way or the other, as long as it is consistent, simple to use, and clear to readers. However, since most of the examples on GitHub does not quote this, people will keep writing the unquoted version, and it will just generate extra work for no specific reason. Is it possible to tweak this?
There was a problem hiding this comment.
Yes, I can disable this particular warning globally. Not sure how useful it is. We don't often add new workflows though.
There was a problem hiding this comment.
We don't often add new workflows though.
Good point. We can also disable it later if it turns out to be a nuisance and cause friction. I have approved the PR, so you can decide what you think is best.
| self.has_subject_body_separator = (len(lines[1].strip()) == 0) | ||
| self.has_subject_body_separator = len(lines[1].strip()) == 0 |
There was a problem hiding this comment.
Strange rule to complain about the first approach since it is a lot easier to read. But if that is what it says...
There was a problem hiding this comment.
I don't remember modifying it, probably that's the black formatter... On the good side, it is not configurable, so we can just give up and live with its minor weirdnesses :)
There was a problem hiding this comment.
Agree. If this is what either of the tools say out of the box, we just roll with it. Just wasn't sure if that was the case.
Helps find some errors and cosmetic problems.
akuzm
force-pushed
the
aku/python-linter
branch
from
7d6280f
to
4422da4
Compare
Helps find some errors and cosmetic problems.
Specifically, I added prospector because it combines many different linters and has lenient defaults. We could also run black on them but that might be too opinionated.
Disable-check: force-changelog-changed