Skip to content
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(flake8): Flake8 does not support 4 letter codes anymore. #119

Merged
merged 4 commits into from
Sep 1, 2022

Conversation

ruler501
Copy link
Contributor

@ruler501 ruler501 commented Aug 21, 2022

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

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
Copy link
Member

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

@ruler501
Copy link
Contributor Author

ruler501 commented Aug 21, 2022

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
@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
Labels
None yet
2 participants