Skip to content

Conversation

kurtmckee
Copy link
Contributor

This commit introduces the following changes:

  • Run pre-commit autoupdate
  • Run pre-commit run -a (no changes made)
  • Update pre-commit config examples to point to latest version
  • Fix a YAML formatting issue in a pre-commit config example

@DanielNoord I'd like to recommend some changes to the pre-commit and linting usage based on these problem points:

  • A script, scripts/lint.sh, is used to run linters.

    This is a manual step that can be missed by PR submitters. It requires users to correctly set up their own virtual environments with all of the correct tools, and excludes Windows users.

  • The script runs in CI, but doesn't fix incoming PRs.

    This means that users must wait for approval to run the GitHub workflows before finding out that a linter is failing.

If you're open to it, I'll submit a PR to migrate some of the linters to the pre-commit config (like black and flake8). However, I recommend enabling pre-commit.ci for this repo; if that's desirable, I'll submit a PR to configure pre-commit.ci to check for hook updates on a quarterly basis.

This commit introduces the following changes:

* Run `pre-commit autoupdate`
* Run `pre-commit run -a` (no changes made)
* Update pre-commit config examples to point to latest version
* Fix a YAML formatting issue in a pre-commit config example
Copy link
Member

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait with making any structural changes to the CI of the project for now.

Making sure dependencies are up to date is good to do, but actually changing how the project "functions" would require more discussion and approval than just mine.

@DanielNoord DanielNoord merged commit 5589413 into PyCQA:main Jan 9, 2025
1 check passed
@kurtmckee kurtmckee deleted the update-pre-commit-hooks branch January 9, 2025 14:56
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