8000 fix(flake8): Flake8 does not support 4 letter codes anymore. by ruler501 · Pull Request #119 · life4/deal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(flake8): Flake8 does not support 4 letter codes anymore. #119

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 1, 2022
Merged

fix(flake8): Flake8 does not support 4 letter codes anymore. #119

merged 4 commits into from
Sep 1, 2022

Conversation

ruler501
Copy link
Contributor
@ruler501 ruler501 commented Aug 21, 2022 •
edited
Loading

See https://github.com/PyCQA/flake8/blob/main/src/flake8/plugins/finder.py#L25 for flake8 code.

I think DEL might also work

@orsinium
Copy link
Member
orsinium commented Aug 21, 2022 •
edited
Loading

Damn >.< Thank you for taking care of it. Do you know a relevant discussion for the change in flake8? I'd like to see what motivated them to break plugins again.

I'm going to take some time and think about the best error code to use now (1773-style DEA1?). And how to do backward compatibility from our side. If we change the code, the old noqa: DEAL won't work. Maybe, it's not a big deal, though.

Also, looks like we need an integration test with flake8, since they like breaking things. But don't worry, I can take care of it.

@orsinium orsinium mentioned this pull request Aug 21, 2022
Merged
@orsinium
Copy link
Member

What do you think about using a heterograph, like DIL? 🙃

@ruler501
Copy link
Contributor Author
ruler501 commented Aug 21, 2022 •
edited
Loading

That seems to work. I think I like DEL more though. DEA1 is also good

@ruler501
Copy link
Contributor Author

PyCQA/flake8#325 seems to be the source of our troubles. Still not sure why they chose to restrict to 3 characters.

@ruler501
Copy link
Contributor Author

Merged your integration test in to see if everything works

@orsinium
Copy link
Member

Make it DEL, please, and I'll be happy to merge. Meanwhile, I'll have a look if I can patch the Checker to recognize the old noqa comments. I don't want the changes to be breaking, so I'll provide a backward compatibility bridge.

@orsinium orsinium mentioned this pull request Aug 26, 2022
Merged
@orsinium
Copy link
Member
orsinium commented Sep 1, 2022

Looks like I have to take it from there 👀

@orsinium orsinium merged commit 8200971 into life4:master Sep 1, 2022
@orsinium
Copy link
Member
orsinium commented Sep 1, 2022

Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ruler501 @orsinium
0