Skip to content

style: fix target Python version for Black pre-commit hook#431

Merged
agronholm merged 1 commit into
pypa:mainfrom
henryiii:henryiii/style/fixpyver
Dec 29, 2021
Merged

style: fix target Python version for Black pre-commit hook#431
agronholm merged 1 commit into
pypa:mainfrom
henryiii:henryiii/style/fixpyver

Conversation

@henryiii
Copy link
Copy Markdown
Contributor

@henryiii henryiii commented Dec 29, 2021

This fixes a bug in the pre-commit configuration - language-version controls the pre-commit hook, not the black language target. So this was actually targeting the black default language version (2.7 or 3.6 or generic), and it was only working on a machine that had Python 3.7 installed.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 29, 2021

Codecov Report

Merging #431 (482c3a8) into main (5a8ae8d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #431   +/-   ##
=======================================
  Coverage   66.88%   66.88%           
=======================================
  Files          11       11           
  Lines         897      897           
=======================================
  Hits          600      600           
  Misses        297      297           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a8ae8d...482c3a8. Read the comment docs.

@henryiii
Copy link
Copy Markdown
Contributor Author

This is changing docs/conf.py - we can add it to the ignores if you would prefer it not be changed.

@henryiii henryiii changed the title style: fix target Python version for Black hook style: fix target Python version for Black pre-commit hook Dec 29, 2021
@henryiii henryiii mentioned this pull request Dec 29, 2021
2 tasks
@agronholm
Copy link
Copy Markdown
Contributor

Why are non-Python files affected?

@henryiii
Copy link
Copy Markdown
Contributor Author

henryiii commented Dec 29, 2021

Because pre-commit/pre-commit-hooks (end-of-file-fixer, one of the checks at the top) isn't being run in CI.

@agronholm
Copy link
Copy Markdown
Contributor

Yeah, but pre-commit hooks only run on the files that have changed. Those .rst files weren't supposedly altered by this change.

@henryiii
Copy link
Copy Markdown
Contributor Author

I always run pre-commit run -a, which runs on all files. That's also what you are supposed to be running in CI if you use pre-commit (and what pre-commit.ci and pre-commit/action will do). The only-changed part is just for performance, it's not supposed to be invalid, otherwise changing that file will suddenly cause unrelated whitespace changes.

@agronholm
Copy link
Copy Markdown
Contributor

Interesting. While researching I found that the pre-commit Github action is currently unmaintained and directs to https://pre-commit.ci/ instead. Should I enable that service for this repo?

@henryiii
Copy link
Copy Markdown
Contributor Author

I'd recommend it, I really like it (and it's enabled for a few of the other PyPA repos).

@agronholm
Copy link
Copy Markdown
Contributor

Ok, it's enabled now.

@henryiii henryiii closed this Dec 29, 2021
@henryiii henryiii reopened this Dec 29, 2021
@henryiii
Copy link
Copy Markdown
Contributor Author

Took 6.9 seconds. :)

@agronholm agronholm merged commit 108b9f6 into pypa:main Dec 29, 2021
@henryiii henryiii deleted the henryiii/style/fixpyver branch December 29, 2021 22:14
@agronholm
Copy link
Copy Markdown
Contributor

I should nix the lint build stage as it's redundant now.

@agronholm
Copy link
Copy Markdown
Contributor

Thanks for this PR and all the extra info. This was quite enlightening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants