-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(F822): Enable F822 in __init__.py files by default #11370
Conversation
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
F822 | 12 | 12 | 0 | 0 | 0 |
I'm wondering if we should instead always enable this, and ask users to add |
I.e., if we think tis should be the default, maybe we just make it the default and use the existing |
621369e
to
31f6f9b
Compare
Thanks for taking a look at this @charliermarsh. I've changed the PR to simply remove the special handling of F822 warnings from |
@charliermarsh just a gentle reminder 😊 |
Could you make this change only apply when preview mode is enabled so we don't increase the scope of the rule without a stabilization period? I think it'd be |
31f6f9b
to
a8dda84
Compare
a8dda84
to
686ef15
Compare
@zanieb apologies for the slow reply, it's been a busy work week so far. I've made the suggested change and added a test for the preview to make sure the preview enabled scenario is tested too. How long do changes like this usually have to be in preview, before they can become stable? |
Thanks @hassec. Will take a look shortly. We tend to leave a change in preview for one minor release (so this would be elevated in v0.6.0, since v0.5.0 will go out in the next few weeks). |
CodSpeed Performance ReportMerging #11370 will improve performances by 7.9%Comparing Summary
Benchmarks breakdown
|
Hey thanks for the change! Just a nitpick as a user about the changelog entry, it says
So I went looking for the new option, even reading this PR's description and looking for |
Thanks, good call! |
NOTE
The actual implementation of this PR changed during review, the below description is outdated.
Summary
This PR aims to close #10095 by adding an option
init-allow-undef-export
to thepyflakes
settings. This option is currently set totrue
such that behavior is kept identical.But setting this option to
false
will lead toF822
warnings to be shown in all files, including__init__.py
files.As I've mentioned on #10095, I think
init-allow-undef-export=false
would be the more user-friendly default option, as it creates fewer surprises. @charliermarsh what do you think about making that the default?With this option in place, it's a single line fix for people that rely on the old behavior.
And thinking longer term, for future major releases, one could probably consider deprecating the option and eventually having people just
noqa
these warnings if they are not wanted.Test Plan
I've added a
test_init_f822_enabled
test which repeats the test that is done in theinit
test but this time withinit-allow-undef-export=false
and the snap file correctly shows that ruff will then trigger the otherwise suppressed F822 warning.closes #10095