Merged
Conversation
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
approved these changes
Jan 9, 2025
Member
DanielNoord
left a comment
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This commit introduces the following changes:
pre-commit autoupdatepre-commit run -a(no changes made)@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.