Open dgw opened 4 years ago
I tend to completely ignore the "unused argument" warning in my code: sometimes, you need that argument in your interface, but not every implementation will need it, or even maybe most of them won't. It's OK, it's part of the life of an interface and its implementations.
I think there are better way to improve the code quality than checking that, and adding # noqa: U100
all over the place is not really my cup of tea.
My suggestion: we are all done for 7.1 as far as I'm concerned.
I still have a WIP branch for flake8-docstrings, and I don't intend to ship 7.1 without finishing it.
Unused args, fine, that decision can wait until after 7.1 and we check how many # noqa
s we'd need.
aaaaaaaaaaaaa You should comment on issues with draft stuff you've worked on, cause I just spent most of the evening on it again. Though yours looks like a challenging rebase anyway by now
I did self-assign this issue… But if you're considerably further along than mine (and I'm sure you are), don't worry about it. Especially now that I know someone else is working on it, I'm not gonna go finish mine and open a PR to snipe you.
I'm looking at your branch @half-duplex and I really don't like this syntax:
"""
Synopsis of multi-line docstring.
Long description.
"""
I don't see the need for that change.
Also, I don't see the point of adding short docstring just for the sake of having a docstring.
Also, if you want to flag stuffs without docstring: use pylint.
I'm looking at your branch @half-duplex and I really don't like this syntax
I wasn't going to break """Synopsis
lines, but dgw's branch added that warning.
Also, I don't see the point of adding short docstring just for the sake of having a docstring.
Options: Disable missing docstring warning (ehh). noqa
them (ehh). Or add a short, dubiously-useful docstring that can be filled out as needed (eh).
pydocstyle also doesn't seem to find superclass stub method docstrings, so I'm not sure how to deal with that. Right now I'm adding noqa
s as https://github.com/PyCQA/pydocstyle/issues/309 mentions, since duplicating docstrings, breaking docs and introspection with """Override method."""
, or adding a dependency and ignoring all checks for @override
methods all seem worse.
I wasn't going to break
"""Synopsis
lines, but dgw's branch added that warning.
It's a WIP, and shouldn't be treated as gospel. I don't like it either, and would have changed it myself before PR'ing.
Yeah, it seems to me that what we want for now is just the pylint
report.
When improving the code quality, it's often best to do that in 3 steps:
pylint
will do in that caseFlake 8 is best at step 3, because we rely on it to validate the code before merge & packaging. Pylint is best for 1 & 2 as it generate a report you can use from time to time to know what's wrong and what you can fix.
I'm going to leave the rest of this stuff for 8.0. We put in a ton of doc/docstring work for 7.1, and I'd rather go over things again to fix warnings after we remove the deprecated stuff that's due for deletion in 8.0. 🙃
In case my closing note for #1978 wasn't clear enough, the target approach for docstrings at this moment is going to be those first 2 steps @Exirel mentioned. We'll probably want to avoid monolithic PRs for any of these, which means I fully expect the milestone to float along as we decide stuff is ready to ship and move to the next version.
I've added flake8-quotes in #2322.
Because this won't affect the software in any way, I've decided it needn't be part of any milestone. "Tracking" and "Long-term Planning" labels both indicate it's more of a wishlist, not part of any specific roadmap.
I have a list of flake8 plugins I'd like to add after we release 7.0:
flake8-future-import
for easier enforcement of the required__future__
imports [#1561]flake8-import-order
to check import sorting (kind of redundant with the existingisort
project in #1752, but has the benefit of being integrated with our current checks;in-progress by me, @dgw, in a personal branch nameddone by @half-duplex in #1864)flake8-import-order
flake8-unused-arguments
to do what it says on the tin (we can always ignore specific instances for e.g. backward compatibility with# noqa: U100
comments)flake8-comprehensions
for sanity checks—these were some of the DeepSource warnings that I thought were actually worth looking at from #2016[ ]Removed/NAK as the plugin can't be configured to enforce the desired style choices.flake8-quotes
to push us toward using single-quotes except when the string contains them (my own preference) (#2322)flake8-requirements
for core and plugins (as we extract stuff into separate packages), to make sure dependencies stay saneflake8-string-format
to check.format()
calls—though it mightn't be maintained any more? and/or there could be places where we intentionally break the conventions for stuff like pluralizing messagesflake8-type-checking
to guard against runtime overhead from importing modules that only appear in type annotations (#2300)flake8-docstrings
to enforce blank lines (or not) around docstrings, and flag things that aren't documented but should beflake8-rst-docstrings
to lint reStructuredText docstrings, keeping our docs more consistentThere are probably more plugins that would be useful, which anyone may suggest here. I'll modify this checklist as needed.