Add a yapf precommit hook #72
Conversation
hooks/pre-commit
Outdated
|
|
||
| readonly changed_files_command="git diff --cached --name-only | grep '\.py$' | grep -v 'servers/[^\/]*/jobs/models' | grep -v 'setup.py'" | ||
| readonly files_in_commit=$(eval $changed_files_command) | ||
| echo $files_in_commit |
There was a problem hiding this comment.
I prefer not to have all these echo lines (line 19, 22, 28) for every commit.
Ideally precommit would automatically format and save files, without manual intervention, like "pretty-quick --staged". Since this PR doesn't do that, I would like to either cancel this PR, or remove all this logging.
| @@ -13,3 +13,22 @@ then | |||
| echo Please commit to a working branch and push it to GitHub for review/merge. | |||
| exit 1 | |||
| fi | |||
|
|
|||
| readonly changed_files_command="git diff --cached --name-only | grep '\.py$' | grep -v 'servers/[^\/]*/jobs/models' | grep -v 'setup.py'" | |||
There was a problem hiding this comment.
Why do you exclude these files, like setup.py? The list of files to exclude should be kept in sync with CircleCI. CircleCI only excludes ui/node_modules.
| if ! [ -z "$yapf_out" ] | ||
| then | ||
| echo "Reformatting staged files, please review them and stage the changes." | ||
| yapf -i $files_in_commit --exclude ui/node_modules/ |
There was a problem hiding this comment.
# Keep in sync with .circleci/config.yml
Also add comment to .circleci/config.yml
|
|
||
| if ! [ -z "$yapf_out" ] | ||
| then | ||
| echo "Reformatting staged files, please review them and stage the changes." |
There was a problem hiding this comment.
I just tested this out.
- Made a change to a file that would cause yapf to fail
- git commit -m 'blah'
- commit went through with incorrectly formatted file
- After commit, file is still incorrectly formatted.
That's not what you expect, right?
My advice is to abandon this PR, rather than try to get this working.
If you can get the automatic reformatting working, then this PR is worth it. Seems doable given you have file_in_commit.
|
Facebook is working on a new Python formatter, Black, which they will use for their .py files. I think Black will have more adoption than YAPF over time. I prefer Black over YAPF because:
See this thread for more details. What do you think about looking into Black? |
No description provided.