Skip to content

Format and check all Python code using python black#1776

Merged
wohali merged 2 commits intomasterfrom
python-black
Dec 6, 2018
Merged

Format and check all Python code using python black#1776
wohali merged 2 commits intomasterfrom
python-black

Conversation

@wohali
Copy link
Copy Markdown
Member

@wohali wohali commented Nov 29, 2018

Inspired by our formatting work on Elixir, this PR proposes doing the same thing for our Python code.

Python Black is an opinionated code formatter now adopted by prominent Python projects such as pip. It is easy to use and provides both formatting and check modes.

The python-black target builds a python3 venv at .venv and installs black if possible. Since black is Python 3.6 and up only, we skip the check on systems with an older Python 3.x and print a warning. Our CI systems will still catch problems from people who submit PRs who don't have 3.6+ since CI is configured to run this check automatically on at least one platform (e.g., Jenkins Ubuntu Bionic, or Python 3.6 explicitly requested under Travis CI.)

Assuming the check runs, it fails if any file would be changed by the formatter.

A Makefile target is provided that actually does the updating (make python-black-update, it simply omits the --check flag) for happy dev fingers to fix up python files.

The change to the docs dependency in rebar.config.script is necessary because of embedded Python files in that repo that have only just been re-formatted. Once docs is tagged with 2.3.0, this dep will be reverted to a tag-type reference, not a commit.

@nickva I had to change the location of the Mango venv to src/mango/.venv so it would be automatically ignored by the formatter, otherwise it tries to format the entire venv. Oops :)

Testing recommendations

If you have Python 3.6+, try make python-black and see if it works.

if you don't, try the same command and make sure it is a noop (with an exit code of 0).

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

@wohali wohali requested review from davisp, janl and tonysun83 November 29, 2018 04:46
@wohali wohali mentioned this pull request Nov 29, 2018
3 tasks
@wohali wohali force-pushed the python-black branch 4 times, most recently from 160c2ca to be6c902 Compare November 29, 2018 06:33
Comment thread build-aux/logfile-uploader.py
@wohali
Copy link
Copy Markdown
Member Author

wohali commented Dec 1, 2018

Any comments from @janl @davisp @iilyak @eiri or anyone else who does Python-y things? Waiting on a +1 or comments to the contrary.

Comment thread src/couch/compile_commands.json Outdated
Copy link
Copy Markdown
Member

@davisp davisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 Yay consistency!

The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
@wohali wohali merged commit 33d4f6d into master Dec 6, 2018
@wohali wohali deleted the python-black branch December 6, 2018 23:04
janl pushed a commit that referenced this pull request Feb 7, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
janl pushed a commit that referenced this pull request Feb 17, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
AGrzes pushed a commit to AGrzes/couchdb that referenced this pull request Nov 2, 2019
The Makefile target builds a python3 venv at .venv and installs
black if possible. Since black is Python 3.6 and up only, we
skip the check on systems with an older Python 3.x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants