-
Notifications
You must be signed in to change notification settings - Fork 166
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
Add config file for linters #547
Conversation
@edreamleo does Leo observe the flake8/pycodestyle config in setup.cfg? Can we add an ignore rule for some of these rules? I've never been a fan of rule F841, I think while it's useful during development to be warned of unused variables, the fixes/workarounds are more often than not actually makes the code worst. I don't think adding dummy Removing the unused assignment also are often undesirable for a number of reasons:
IMO, even truly unused assignment where the variable name doesn't carry any useful information is a fairly minor issue, it's not really worth the problems caused by trying to avoid them.
|
The first 12 minutes of Raymond Hettinger's Beyond PEP8 was using line-length as an example, but it also applies to F841 as well. |
@lieryan For me this issue is not about specific pip 8 rules. Rather, the issue is whether I can easily use pyflakes. Yes, there is workaround, namely adding an However, I would like to discuss your concerns about unused names further. I agree that the unused names, especially in unit tests, form a reasonable and useful pattern. Adding an assert does not change the pattern! Imo adding Summary None of the settings in the config file affect me (or Leo) in any way. I can live without the new asserts, but it is my strong opinion that making small accommodations to pyflakes is well worth doing for any python project and any python dev. |
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.
@lieryan I'm good with the settings in this file.
@edreamleo can Leo be configured to use flake8 instead of pyflakes for the whole project? flake8 already does all the checks that pyflakes does (it uses pyflakes internally), the main difference is that flake8 adds a mechanism to configure the output, is there any reason Leo is using just pyflakes by itself rather than via flake8? If it's using flake8, it would've picked up the config file. I can be ok with keeping the assertXXX in tests, but I'm much less keen on having the |
@lieryan I'm going to remove the extra asserts discussed here from my future PRs. We both have better things to do than squabble over style! :-) |
@lieryan Leo could use flake8 automatically, but pyflakes works better for me. Or maybe I'm more used to pyflakes. Anyway, I'm going to defer to your preferences for rope. I've also added a private pre-commit script that runs black, reindent and flake8, so we should be on the same page from here on. |
Thanks @edreamleo, I appreciate the understanding. |
@lieryan You're welcome. I'm excited to be part of this project. Learning how to fit in is an essential first step. |
Add config file for linters