-
Notifications
You must be signed in to change notification settings - Fork 23
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
Enable TypeAlias check by default #101
Conversation
Dependent on PyCQA/flake8-pyi#101.
The typeshed-primer failures are all Y026. We'll have to merge python/typeshed#6958 first for CI to become green. |
Dependent on PyCQA/flake8-pyi#101.
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, but I think we should maybe advertise this change more loudly, either in the README or the changelog (and maybe point to instructions elsewhere about how to disable certain codes in your flake8 config file?). If third-party users of this package are anything like typeshed, this check is going to cause a large number of hits.
Yeah, I have a tendency to think of typeshed as the main consumer of flake8-pyi, but that's not really the case. It might be nice to collect the codemods we've applied to typeshed and link them in our CHANGELOG. It could also be nice to update stubgen output to be compliant with all these new lints. |
Linking to the PRs in the CHANGELOG sounds like a great idea. We could maybe also have somewhere to collect some of the scripts we've used for codemodding — either a new repo, or a subdirectory in this repo. WDYT? |
A scripts directory here seems like a decent place, at the very least we're all here now! @hoefling shared an excellent libcst script in python/typeshed#5564 (comment) that we could add on to. |
I don't like the idea of collecting everything into a
I think it would be better to instead improve error messages to say what you should do instead. Some of our error messages already work that way:
This makes writing a script easy enough to just do yourself if you need one. We could even include one in this repo, and if it's simple enough, it shouldn't be too much work to test and maintain. Improving stubgen output would also be nice :) |
I wasn't envisioning writing tests for the scripts. We could have a README file in the subdirectory that has a big "USE WITH CAUTION — UNTESTED" notice in it. I also didn't mean to suggest that we needed to include every codemodding script we'd ever written in the subdirectory. It could be a selection. |
I added a section listing codes users may not want to enable. Your Y027 can also get a mention there. I'd be OK with adding cleanup scripts to this repo, but let's do that in a separate PR. |
agreed. |
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.
LGTM
And delete the machinery for disabling errors.
This made this check run on all tests, which helped me find a bug
(it triggered on all). I can split the test changes into a separate
PR if preferred.
I'll submit a separate PR to typeshed to disable this check for now.
Fixes #75. Fixes #86.