-
Notifications
You must be signed in to change notification settings - Fork 453
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
chore(pre-commit): Fixes configuration of pre-commit #906
Conversation
Switches autopep8_wrapper to autopep8_mirror, no longer available in pre-commit-hooks directly Switches flak8 to pyca/flake8, no longer available in pre-commit-hooks directly Adds black Updates hook versions to latest available Fixes configuration error due to changed schema upstream ``` [WARNING] normalizing pre-commit configuration to a top-level map. support for top level list will be removed in a future version. run: `pre-commit migrate-config` to automatically fix this. [WARNING] Unexpected key(s) present on https://github.com/pre-commit/pre-commit-hooks: sha ``` Signed-off-by: miigotu <miigotu@gmail.com>
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.
Thanks for the contribution! I gave this configuration a shot locally and there're some things that don't quite work for this repo right now.
- repo: https://github.com/python/black | ||
rev: 22.8.0 | ||
hooks: | ||
- id: black |
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 repo doesn't presently use black
for formatting, so please don't add it here.
additional_dependencies: [ | ||
'flake8-bugbear==19.8.0', | ||
'flake8-coding==1.3.2', | ||
'flake8-comprehensions==3.0.1', | ||
'flake8-debugger==3.2.1', | ||
'flake8-deprecated==1.3', | ||
'flake8-docstrings==1.5.0', | ||
'flake8-isort==2.7.0', | ||
'flake8-pep3101==1.2.1', | ||
'flake8-polyfill==1.0.2', | ||
'flake8-print==3.1.4', | ||
'flake8-quotes==2.1.1', | ||
'flake8-string-format==0.2.3', | ||
] |
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.
These additional flake8 dependencies could be added later, but they're not currently in use.
Having them in yields some 2000 errors.
default_language_version: | ||
python: python3.7 |
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.
I don't think this is necessary. We should be able to run pre-commit with other versions of Python. (With this, I get RuntimeError: failed to find interpreter for Builtin discover of python_spec='python3.7'
.)
'flake8-debugger==3.2.1', | ||
'flake8-deprecated==1.3', | ||
'flake8-docstrings==1.5.0', | ||
'flake8-isort==2.7.0', |
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.
There seems to be something wrong with this version of flake8-isort:
ImportError: cannot import name 'SortImports' from 'isort'
args: ['--django'] | ||
- id: check-json | ||
- id: check-yaml | ||
- id: debug-statements |
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 should also exclude tests/messages/data
since there's still (not that it's necessarily relevant) Python 2 code in there.
- repo: https://github.com/pre-commit/mirrors-autopep8 | ||
rev: 'v1.7.0' | ||
hooks: | ||
- id: autopep8 | ||
exclude: (docs/conf.py|tests/messages/data/) |
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.
autopep8 seems to make changes that conflict with flake8
, so I'd suggest adding
[flake8]
max-line-length = 120
ignore = E731,W503,W504
to setup.cfg
, which leaves us with a handful (27) manual fixes to do...
'flake8-quotes==2.1.1', | ||
'flake8-string-format==0.2.3', | ||
] | ||
exclude: (docs/conf.py|babel/messages/__init__.py|babel/__init__.py|tests/messages/data|scripts/import_cldr.py) |
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.
I think
exclude: (docs/conf.py|babel/messages/__init__.py|babel/__init__.py|tests/messages/data|scripts/import_cldr.py) | |
exclude: (docs/conf.py|tests/messages/data/) |
should be enough, just like for autopep8
..?
I'll try and get back in this one and make the requested changes within the next day or so. |
Closed via #949 (which uses |
Switches autopep8_wrapper to autopep8_mirror, no longer available in pre-commit-hooks directly Switches flak8 to pyca/flake8, no longer available in pre-commit-hooks directly Adds black
Updates hook versions to latest available
Fixes configuration error due to changed schema upstream
Signed-off-by: miigotu miigotu@gmail.com