Skip to content
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: Move codespell script to pre-commit #88969

Closed
wants to merge 1 commit into from

Conversation

akien-mga
Copy link
Member

@akien-mga akien-mga commented Feb 28, 2024

Makes it enabled by default in pre-commit hooks, which should help users catch typos.

Opening as a draft as I think this risks being a bother to users, as codespell can sometimes have false positives, and it won't let them commit before solving the false positive or adding it to the ignore list.

On the other hand I raised its quietness option, which means it no longer nags about potential issues that it spotted but refused to fix as those fixes are disabled by default (due to often being false positives). So the remaining "sure" fixes might not be too bad.

One option could be to add this but leave it opt-in, by using stage: [manual], so it doesn't actually run automatically on pre-commit hooks. It then needs to be run manually with pre-commit run codespell --hook-stage manual... seems a bit tedious too.

Another drawback to the approach in this PR is that we lose the PR diff annotations on GitHub, but I'm not sure how useful they were as they were not blocking, and we still introduced typos regardless.

Edit: This may be a nicer approach to set the file exclude and words ignore lists: https://github.com/codespell-project/codespell/blob/master/pyproject-codespell.precommit-toml

Makes it enabled by default in pre-commit hooks, which should help
users catch typos.
@akien-mga akien-mga added this to the 4.x milestone Feb 28, 2024
@akien-mga akien-mga changed the title Pre-commit: Move codespell script to pre-commit Pre-commit: Move codespell script to pre-commit Feb 28, 2024
@akien-mga
Copy link
Member Author

Superseded by #91597.

@akien-mga akien-mga closed this May 8, 2024
@akien-mga akien-mga removed this from the 4.x milestone May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant