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

ci: Add spell checker in pre-commit #4859

Merged
merged 12 commits into from
Feb 27, 2025
Merged

ci: Add spell checker in pre-commit #4859

merged 12 commits into from
Feb 27, 2025

Conversation

vedpawar2254
Copy link
Contributor

Fixes #2971

I added a spell checker to the pre-commit, please test it out and see if it's good, i can update if needed.
Also there are a few spelling mistakes that this is giving me, I'll fix them and raise a pr in a bit
Thanks

@terriko
Copy link
Contributor

terriko commented Feb 26, 2025

It's not clear to me if this actually addresses #2971 because our current spell checker runs on every word and this seems like it's got more of a typo-specific thing going on, but it seems like it could be a good supplement nonetheless.

@vedpawar2254
Copy link
Contributor Author

Yeah I guess you are right, I tried with check-spelling but seems to have some docker thing and meanwhile I came across this and thought it would't hurt.
I am still not sure how to do the check-spelling one though if could help me out a little there that would nice.

@vedpawar2254 vedpawar2254 changed the title [Ci] Add spell checker in pre-commit CI: Add spell checker in pre-commit Feb 26, 2025
Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Alas, I don't know any obvious alternatives, so it's a research project for someone!

Anyhow, this looks like codespell is not failing when we run it, so it's probably safe to try it out on main and see if we find it helpful. Before I merge this, can you put a few lines explaining it into https://github.com/intel/cve-bin-tool/blob/main/CONTRIBUTING.md#running-linters -- just a line explaining what it's for and then a little section explaining how to run it on the command line so people can cut/paste if they want to run it before pre-commit triggers. You can see what the other ones look like there.

@terriko terriko changed the title CI: Add spell checker in pre-commit ci: Add spell checker in pre-commit Feb 26, 2025
@vedpawar2254
Copy link
Contributor Author

I guess this should be good

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Minor nit with explicitly setting the version number in requirements.txt, but I'll accept my own suggestion and then merge this once the tests are done. Thank you again for your time researching and setting this up!

@terriko terriko enabled auto-merge (squash) February 27, 2025 18:37
@vedpawar2254
Copy link
Contributor Author

Thanks @terriko

@terriko terriko merged commit 9cf2ff0 into intel:main Feb 27, 2025
21 of 23 checks passed
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.

CI: spell check in pre-commit
4 participants