-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
Format code and docs with black+blacken-docs+pre-commit #244
Conversation
sloria
commented
Jul 9, 2018
•
edited
Loading
edited
- Format code with black
- Format doc code blocks with blacken-docs. This uncovered a number of syntax issues in the docs.
- Run both with pre-commit
c2574cb
to
ff77a97
Compare
@asottile If you're feeling generous, feel free to give this a drive-by =). |
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.
Looks good! Just a few small things I noticed :)
docs/conf.py
Outdated
sys.path.insert(0, os.path.abspath('..')) | ||
import webargs | ||
sys.path.insert(0, os.path.abspath("..")) | ||
import webargs # flake8: noqa |
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.
fun story about # flake8: noqa
I believe you want # noqa
here (or # flake8: noqa
on a line by itself if you actually don't want to lint this file)
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.
Ah, TIL. Thanks.
setup.cfg
Outdated
max-line-length = 100 | ||
exclude = .git,.ropeproject,.tox,build,.direnv,__pycache__ | ||
ignore = E203, E266, E501, W503, E302 | ||
max-line-length = 80 |
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.
black I believe uses 88
, though I see you've ignored E501
so I guess this line does nothing! :D
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.
Is it possible to use explicit names in there?
-ignore = E203, E266, E501, W503, E302
+ignore = rule-number-one, another-rule
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.
unfortunately not -- pylint has this but flake8 does not
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.
black I believe uses 88, though I see you've ignored E501 so I guess this line does nothing! :D
This is from the recommended flake8 config from black. E501 is ignored, but Bugbear's B901 is enabled, which will warn when the max-line-length exceeds 10% of max-line-length
.
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.
are you using flake8-bugbear
? I don't see it installed
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.
Ah, woops. Forgot to add that to dev-requirements.txt. Will fix this up after work today.
tasks.py
Outdated
|
||
@task | ||
def precommit(ctx): | ||
ctx.run("pre-commit run --all-files", echo=True) |
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.
--show-diff-on-failure
is usually a good option for CI's sake (helps drive-by contributors that might not have set up the linting locally)
My local environment is Python 3.5. Will this make |
Not necessarily, the hooks here are configured with |
Addressed @asottile 's comments and also added some notes in CONTRIBUTING.rst about needing the
|
OK. This means that all contributors on Python 3.5 won't be able to use There's a trade-off, here. This commit adds more features to No big deal. If I can't run it locally, Travis will complain and I'll use my special ability ( |
@lafrech You can use Python 3.5 in your virtualenv and still use pre-commit as long as you have the With pyenv:
|