GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-109408: Move the Python file whitespace check from patchcheck to pre-commit by AA-Turner · Pull Request #109891 · python/cpython

Conversation

@AA-Turner

@AA-Turner AA-Turner commented Sep 26, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Move Azure Pipelines CI to GitHub Actions #109408

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
AA-Turner added 2 commits September 26, 2023 11:27
@AA-Turner
Move the Python file whitespace check to pre-commit
606aa73
@AA-Turner
Unused function
d7160fd
@AA-Turner
AA-Turner requested review from AlexWaygood and hugovk as code owners September 26, 2023 10:36
@bedevere-app bedevere-app Bot added the awaiting review label Sep 26, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Sep 26, 2023
Move Azure Pipelines CI to GitHub Actions #109408
Closed
@AA-Turner AA-Turner added the skip news label Sep 26, 2023
AA-Turner added 2 commits September 26, 2023 11:38
@AA-Turner
Minor pattern update
4adbd0d
@AA-Turner
quotes
7b99698
This was referenced Sep 26, 2023
gh-109408: Run patchcheck in GitHub Actions #109459
Closed
gh-109408: Stop running patchcheck in CI #109895
Merged
AlexWaygood
AlexWaygood approved these changes Sep 26, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Sep 26, 2023
hugovk
hugovk approved these changes Oct 10, 2023
View reviewed changes
@hugovk
Merge branch 'main' into patchcheck/python-file-whitespace
9774833
@hugovk
hugovk enabled auto-merge (squash) October 10, 2023 08:46
@hugovk

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk
hugovk merged commit 08ec4a1 into python:main Oct 10, 2023
@bedevere-app bedevere-app Bot removed the awaiting core review label Oct 10, 2023
@hugovk hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 10, 2023
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

Thanks @AA-Turner for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

Thanks @AA-Turner for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
Contributor

Sorry, @AA-Turner and @hugovk, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 08ec4a1dbf66383303de9ce5cb55b2b437ef92c0 3.11
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned hugovk Oct 10, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 10, 2023
@AA-Turner @hugovk @miss-islington
pythonGH-109408: Move the Python file whitespace check from patchchec…
cbf7a0b 
…k to pre-commit (pythonGH-109891)

(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 10, 2023

Copy link
Copy Markdown

GH-110633 is a backport of this pull request to the 3.12 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Oct 10, 2023
AlexWaygood pushed a commit that referenced this pull request Oct 10, 2023
@miss-islington @AA-Turner @hugovk
[3.12] GH-109408: Move the Python file whitespace check from patchche…
9485765 
…ck to pre-commit (GH-109891) (#110633)

GH-109408: Move the Python file whitespace check from patchcheck to pre-commit (GH-109891)
(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@hugovk

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

@AA-Turner Please could you take care of the backports in #109890 and here?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
AA-Turner added a commit to AA-Turner/cpython that referenced this pull request Oct 10, 2023
@AA-Turner
[3.11] pythonGH-109408: Move the Python file whitespace check from pa…
26ccc48 
…tchcheck to pre-commit (pythonGH-109891)

Co-authored-by: Hugo van Kemenade <[email protected]>.
(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 10, 2023

Copy link
Copy Markdown

GH-110641 is a backport of this pull request to the 3.11 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Oct 10, 2023
@hugovk

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

I've found a problem with this (and the C one) when running on macOS, pre-commit cannot find an executable called python:

❯ pre-commit run --all-files
Run Ruff on Lib/test/....................................................Passed
Run Ruff on Argument Clinic..............................................Passed
check toml...............................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
Check Python file whitespace.............................................Failed
- hook id: python-file-whitespace
- exit code: 1

Executable `python` not found

Check C file whitespace..................................................Failed
- hook id: c-file-whitespace
- exit code: 1

Executable `python` not found

Sphinx lint..............................................................Passed
Check hooks apply to the repository......................................Passed
Check for useless excludes...............................................Passed

It works with python3, but that's not going to work with Windows.

Is there a portable solution?

One option could be like https://github.com/pre-commit/pre-commit-hooks/blob/main/.pre-commit-hooks.yaml which has entries such as:

-   id: check-added-large-files
    name: check for added large files
    description: prevents giant files from being committed.
    entry: check-added-large-files
    language: python
    stages: [commit, push, manual]

Which uses a file like https://github.com/pre-commit/pre-commit-hooks/blob/main/pre_commit_hooks/check_added_large_files.py
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Oct 10, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

(So let's hold off on the remaining backports until this is fixed.)
AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AA-Turner

AA-Turner commented Oct 10, 2023

Copy link
Copy Markdown
Member Author

The pre-commit website (implicitly) suggested using python for system languages, I hadn't realised it wouldn't work on Macs.

The drawback with language: python is that it seems there's no option to run outwith a venv, so we'd suffer an unnecessary slowdown.

A
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AlexWaygood AlexWaygood mentioned this pull request Oct 10, 2023
gh-110652: Fix pre-commit hooks on macOS #110653
Closed
This was referenced Oct 10, 2023
The pre-commit hooks do not work on macOS #110652
Closed
gh-109408: Revert pre-commit whitespace checks pending portable solution #110726
Merged
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@AA-Turner @hugovk @Glyphack
pythonGH-109408: Move the Python file whitespace check from patchchec…
2d6f00d 
…k to pre-commit (python#109891)

Co-authored-by: Hugo van Kemenade <[email protected]>
@AlexWaygood AlexWaygood mentioned this pull request Oct 19, 2024
Meta: Run check-peps.py in pre-commit python/peps#4071
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes
@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@hugovk hugovk

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@AA-Turner @hugovk @miss-islington @AlexWaygood
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.