Running the black code formatter on the codebase #241
Conversation
|
To add: I do want to put in some pre-push git hook to prevent the code from being de-blackified, but I've also found that it's good to do this a bit after blackifying the codebase, so as to not stir things up too much. Once (if?) we do add such a hook (for example via the pre-commit.com mechanism), there might be some files that will have gone out-of-black format, but it's no big deal to blackify them along with adding the hook. |
tomage
force-pushed
the
format-with-black
branch
2 times, most recently
from
419d539
to
f5b8551
Compare
Makefile
Outdated
| @@ -82,6 +82,11 @@ update-dependencies: | |||
| @make freeze-dependencies | |||
|
|
|||
|
|
|||
| .PHONY: format | |||
| format: setup | |||
| @. .venv/bin/activate && black . | |||
There was a problem hiding this comment.
Decided to call the target format, and not black, as there might be other formatting tools we might want to add (JS/HTML/CSS..?).
| @@ -62,19 +66,34 @@ def get_answer(question, proper_answers=('yes','no')): | |||
| return answer | |||
|
|
|||
|
|
|||
| print "*" * TERMINAL_WIDTH | |||
There was a problem hiding this comment.
I converted these python 2 style print statements into calls to the print() function in python 3.
| line-length = 79 | ||
| # NOTE: PEP8 does recommend double-quotes, but to start with we'll support | ||
| # single quotes as to minimize the patch/noise produced by introducing black | ||
| skip-string-normalization = true |
There was a problem hiding this comment.
I decided to go for 79 chars, as it's by the PEP8 standard.
Also, I figured - for now - to allow using single quote strings. We can change it later if we want to.
| @@ -28,6 +28,9 @@ requests==2.22.0 | |||
| signxml==2.7.2 | |||
| suds-jurko==0.6 | |||
|
|
|||
| # Development | |||
| black==22.3.0 | |||
|
|
|||
| cachetools==4.0.0 | ||
| certifi==2019.11.28 | ||
| cffi==1.13.2 | ||
| chardet==3.0.4 | ||
| click==8.1.3 |
There was a problem hiding this comment.
This, and the other dependencies below came in with the black package.
EDIT: This is really a tiny PR - should take about 5 mins to review I promise :)
Right.. This is a big one! But also, a small one.
It's big because it changes a lot of files, and it may introduce coding styles foreign to some developers.
It's small because functionally, the code is exactly the same. Black is written by one of the core contributors to python, and he did a clever thing with black: It compares the AST (abstract syntax tree) of the code before and after, and makes sure it's the same. This pretty much makes sure that the code functions the same. Hence, this patch is actually small.
**The only files that need any reviewing here are:
formattargetprint()calls) and ran black on it, as all other.pyfiles.pyproject.tomlis nice. It can be used to configure other aspects of our python projects later on. Read more about it here: https://peps.python.org/pep-0621/ and here: https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/.All of the other files are
.pyfiles changed by black, and thus need no reviewing, unless if the reader is curious howblackis formatting the files.