-
-
Notifications
You must be signed in to change notification settings - Fork 592
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
Consider enforcing flake8, black and isort #401
Comments
I'm used to flake8 and isort by working on Django, but I'm not happy at all with black (even if it will come to Django too). At some point, with the accumulation of those code "helper" tools, I'm feeling like that's imposing some sort of "slavery" on developers. However, I understand this is not my project and will accept what the majority prefers. |
I like Black, as I feel like it's releasing developers from having to worry about any formatting discussions. Just run a tool and it's done automatically. (Same for isort.) I've also started using pre-commit too, so you don't need to worry about it. A good thing about that, if you don't like pre-commit, it's completely optional and developers aren't required to use it. And it can be a really handy way to group linters together into a single command on the CI (for example, see linting in tox.ini at https://github.com/hugovk/pypistats/). |
I've started using black+flake8+isort with pre-commit hooks in django projects. Things are better now! :) |
Please see PR #411. |
I ran flake8 on the |
I thought flake8 was run through pre-commit. @hugovk, isn't it the case? |
No, we fixed some Flake8 things but because we decided not to use Black there were still some formatting-related ones to fix. So a PR to fix more would be welcome! And let's add Flake8 to pre-commit. We don't necessarily have to fix all the Flake8 right now (but it'd be nice!), we can temporarily exclude some rules and fix those later. It would help prevent new ones slipping in. |
Thanks! |
I like Django's limit (119), but some judge it too high. |
That is a bit too high for me :) I often diff two things side by side, that would cause a lot of wrapping. Shall we use 88? It's also the Black default, to make things easier in case we decide to adopt it in the future? Still bigger than the Flake8 default 79. |
99 and call it a day (I like selling carpets 🤓 ) |
Deal! |
As part of #398 I've added disabled calls to flake8 including a commented-out installation of the flake8-black and flake8-isort plugins.
I would suggest to run black and isort once over the code base and enabling both plugins to have a positive long-term effect on code quality. I didn't make this part of #398 since it's not a hard requirement for Jazzband projects (yet) but I've seen more and more projects move to that strategy.
The text was updated successfully, but these errors were encountered: