-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add pre-commit hook config #2963
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2963 +/- ##
=======================================
Coverage 98.32% 98.32%
=======================================
Files 69 69
Lines 23230 23230
=======================================
Hits 22842 22842
Misses 388 388
|
Thanks @stefmolin I'll probably wait to engage with this until after the 0.12.0 release but it looks cool. |
Thanks @stefmolin for setting this up. TBH I don't love how you end up with two sources of truth on what will be enforced vs. checked, but it's limited enough in scope to be manageable. Can you please rebase (so you pick up #3009) and then add If you want to add a sentence in the README about needing to do Also it looks like there's a handful of whitespace things that get caught when running this on all files. Do you want to run that and then commit the result as part of this PR too? |
b1a0ebc
to
326f851
Compare
Do you say this because there are additional checks here compared to
All done. |
Oh I just mean that within the lint/type checks, there's some duplication of how files get selected for / excluded from testing. |
I think I understand what you are saying. The files to include/exclude can be specified for all hooks once at the top, but the hooks don't completely overlap in what they are looking at just like the |
Thanks @stefmolin, may continue to tweak over time but expect this to be very helpful! |
Addresses #2950
I'm adding an initial version of a pre-commit config file for contributors to run pre-commit hooks. After testing it out, let me know if you want any changes and whether/where we should put the setup instructions for contributors in the docs.
Setup (from the top-level of the repo):
The next time you
git commit
the hooks will run on the staged files. If there were any changes to the versions of checks in the config file since the last commit or this is your first time using it, it will pull down the specified versions for you:If you don't pass all the checks, the commit doesn't happen, but you can tack on
--no-verify
to yourgit commit
command to bypass the checks.You can test the hooks on files without committing:
Please note that for
flake8
the config option forexclude
does not seem to be honored like the others insetup.cfg
, so I added the equivalent pattern as an exclusion in the YAML file. Here, you can see thatseaborn/cm.py
is ignored for theflake8
check:$ pre-commit run --file seaborn/cm.py check yaml...........................................(no files to check)Skipped fix end of files.........................................................Failed - hook id: end-of-file-fixer - exit code: 1 - files were modified by this hook Fixing seaborn/cm.py trim trailing whitespace.................................................Passed flake8...............................................(no files to check)Skipped mypy.................................................(no files to check)Skipped
Use
--all-files
to run on the full repo:output
This yields some findings:Additional hooks can be found here, and the docs for pre-commit are here.