-
-
Notifications
You must be signed in to change notification settings - Fork 616
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
Start using auto-formatters #754
Conversation
3aff45c
to
b983b61
Compare
@@ -29,6 +29,7 @@ commands = flake8 {posargs} bandit | |||
bandit-baseline -r bandit -ll -ii | |||
|
|||
[testenv:pep8] | |||
ignore_errors = 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.
This is because pylint throws tracebacks if we don't upgrade the version, and spews a bunch of new issues if we do. So for now, let's leave it static and upgrade it in a separate PR
@@ -79,7 +80,14 @@ deps = -r{toxinidir}/doc/requirements.txt | |||
commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html | |||
|
|||
[testenv:pylint] | |||
commands = pylint --rcfile=pylintrc bandit | |||
commands = -pylint --rcfile=pylintrc bandit |
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.
The leading -
tells tox to ignore a non-zero exit code
[testenv:format] | ||
skip_install = true | ||
deps = | ||
pre-commit |
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.
pre-commit is great. It runs things like pyupgrade
the way pyupgrade wants to be run (passing in each file independently). It can be used in local development as well and could run the rest of our linters for us for local development too
@ericwb if you have a chance |
Use black to auto-format the style so it's always consistent and pyupgrade will allow us to auto-upgrade to the newest language features.
b983b61
to
8e056d4
Compare
@lukehinds @ericwb could you two take a gander at this? |
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.
The auto-formatter seems to indiscriminately changing all single quotes to double quote strings. Even though I believe many uses of single quote are appropriate. PEP8 doesn't establish a recommendation (https://www.python.org/dev/peps/pep-0008/#string-quotes), but I see other references that distinguish a difference. (https://www.askpython.com/python/string/difference-between-single-and-double-quotes-in-python). I guess I'm fine with either, but we probably need to document a coding standards doc for the project.
Use black to auto-format the style so it's always consistent and
pyupgrade will allow us to auto-upgrade to the newest language features.