-
-
Notifications
You must be signed in to change notification settings - Fork 8k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pre-commit config + fixes #21638
Conversation
Update pre-commit hooks rst format Disable autofixes on PRs Unless manually triggered. See https://pre-commit.ci/#configuration add pre-commit to environment.yml Slow down autoupdate schedule alphabetize Add sentence about where pre-commit config is Add excludes to pre-commit config Otherwise end of lines of test images will get fixed, this also ignores all the prev_whats_new and prev_api_changes as well as vendored components (agg, gitwash) pre-comit config
examples/images_contours_and_fields/colormap_normalizations_symlognorm.py
Show resolved
Hide resolved
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 the pre-commit install step. You can still run the checks with pre-commit run <list of files> or pre-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 think pre-commit run ... will still update/modify the files if that is what the hook is doing though? It is just if you install 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?
Do we need to have this much white-space churn?
As established when talking about black I'm a supporter of infinite amounts of churn... so that's a question I'm ill suited to answer. I think this rule is already enforced in py files by flake8 and this just extends it too all files. We could change that check to only run on py files.
When tracking what's going on in the codebase, it is nice if "git blame" points to substantive changes rather than whitespace "fixes"
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. I think 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