Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Pre-commit config + fixes#21638
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Update pre-commit hooksrst formatDisable autofixes on PRsUnless manually triggered. Seehttps://pre-commit.ci/#configurationadd pre-commit to environment.ymlSlow down autoupdate schedulealphabetizeAdd sentence about where pre-commit config isAdd excludes to pre-commit configOtherwise end of lines of test images will get fixed,this also ignores all the prev_whats_new and prev_api_changesas well as vendored components (agg, gitwash)pre-comit config
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| @@ -60,6 +60,16 @@ true for ``*.py`` files. If you change the C-extension source (which might | |||
| also happen if you change branches) you will have to re-run | |||
| ``python -m pip install -ve .`` | |||
| Installing pre-commit hooks | |||
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.
Would it be better to talk about using them (ie.pre-commit run) instead of installing them? Personally I prefer not having pre-commit editing my files automatically.
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.
How about adding a discussion of running them with out installing as hooks following this?
Something like:
If you do not want these git hooks you can skip thepre-commit install step. You can still run the checks withpre-commit run <list of files> orpre-commit run --all-files
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.
👍 sounds good to me
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 thinkpre-commit run ... will still update/modify the files if that is what the hook is doing though? It is just if youinstall the hooks automatically get run on every commit without a user running the command explicitly.
👍 to changing this section title to "Using pre-commit hooks" and expanding the text.
Do we need to have this much white-space churn? |
As established when talking about |
When tracking what's going on in the codebase, it is nice if "git blame" points to substantive changes rather than whitespace "fixes" |
But that is only going to happens once, right? Like when merging this. (Or when people use editors that trim whitespaces.) |
needs a rebase so I've put to draft -@ianhi, happy to add this to the weekly call if you want to see if this will go in or not. Ithink we talked about it previously and the general opinion was 👍, but not sure... |
I'm pretty sure all of this has been applied in other PRs now. |
PR Summary
As discussed in#21583 (comment) this PR adds a pre-commit config and then:
PR Checklist
Tests and Styling
flake8-docstringsand runflake8 --docstring-convention=all).Documentation