-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[flake8-simplify] Implement fix for maxsplit without separator (SIM905)
#19851
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-simplify] Implement fix for maxsplit without separator (SIM905)
#19851
Conversation
|
|
Could you rebase your PR now that #19849 is merged. It will make it easier for us to review your changes. Thank you |
f4fe4b9 to
f971443
Compare
ntBre
left a comment
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, this makes sense to me!
If we wanted to be conservative we could probably gate this behind preview since it's conceptually close to adding a new safe fix to a stable rule. However, I think it's okay because the fact that the fix was missing before was kind of surprising. And there are no changes in the ecosystem either.
flake8-simplify] Implement fix for maxsplit without separator (SIM905)
|
The more I think about this, the more I'm leaning towards using preview to be safe. @MichaReiser do you feel strongly one way or the other? I only found 99 results in a code search, so maybe it's quite a niche pattern anyway. The preview-gate itself would be easy enough, but we also need a new function in |
|
I think we have to, according to our versioning policy. See #8074 why we might want to change that. |
|
Sounds good, @RazerM do you mind gating this behind preview? |
|
@ntBre I think I've done it correctly, but let me know. |
ntBre
left a comment
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.
Excellent, thank you!
Stacked on top of #19849; diff will include that PR until it is merged.
Summary
As part of #19849, I noticed this fix could be implemented.
Test Plan
Tests added based on CPython behaviour.