Skip to content

Add codespell integration for spell checking#221

Merged
elevran merged 1 commit intollm-d:mainfrom
Jooho:code_spell
Jul 29, 2025
Merged

Add codespell integration for spell checking#221
elevran merged 1 commit intollm-d:mainfrom
Jooho:code_spell

Conversation

@Jooho
Copy link
Contributor

@Jooho Jooho commented Jul 3, 2025

  • Add codespell tool setup: Added codespell configuration in Makefile.tools.mk with Python virtual environment support
  • Implement check-codespell target: Added new spell checking workflow with smart error handling:
    • Automatically installs codespell in Python virtual environment
    • Saves spelling errors to codespell-results.txt when found
    • Displays errors on console and prompts user to fix them
    • Auto-updates results file on re-run
    • Auto-deletes results file when all errors are fixed
    • Shows success message when spelling check passes
    • Skips deploy/ directory from spell checking

Fix: #127

@vMaroon
Copy link
Member

vMaroon commented Jul 6, 2025

This looks good and useful for other llm-d repositories. I think it makes sense for this PR to include GH Actions (CI) updates too in scope - what do you think?

@nirrozenbaum
Copy link
Collaborator

nirrozenbaum commented Jul 7, 2025

@Jooho can you please rebase to fix the CI issue?
(the issue is now fixed and merged into main)

@elevran
Copy link
Collaborator

elevran commented Jul 7, 2025

This looks good and useful for other llm-d repositories. I think it makes sense for this PR to include GH Actions (CI) updates too in scope - what do you think?

Agree. It should:

  • allow the same spell check solution running both locally (via Makefile) and in CI (via a GH action)
  • allow saving common dictionary in project repo (nice to have: per user dictionary under home directory?)
  • be minimal and lightweight (personally prefer a solution that does not need to pull in an entire Python or JS runtime and packages, if such a solution exists. Single binary if available would be great)

@Jooho is this something you'd like to work on further?

@Jooho
Copy link
Contributor Author

Jooho commented Jul 10, 2025

allow the same spell check solution running both locally (via Makefile) and in CI (via a GH action)
allow saving common dictionary in project repo (nice to have: per user dictionary under home directory?)

I added this part.

be minimal and lightweight (personally prefer a solution that does not need to pull in an entire Python or JS runtime and packages, if such a solution exists. Single binary if available would be great)

There is misspell go binary but I think codespell has more options and the result is better. So I want to use codespell even it is not binary.

@Jooho
Copy link
Contributor Author

Jooho commented Jul 10, 2025

@nirrozenbaum I don't see any conflict now. can you please execute ci?

@Jooho
Copy link
Contributor Author

Jooho commented Jul 11, 2025

@nirrozenbaum It merged master so it should pass all ci this time. can you please execute ci again?

@elevran
Copy link
Collaborator

elevran commented Jul 14, 2025

There is misspell go binary but I think codespell has more options and the result is better. So I want to use codespell even it is not binary.

misspell can be added to the golangci-lint tests and would be helpful in some cases. I agree that it is not a good enough option (e.g., dictionaries are not configurable, AFAICT).
I was thinking more along the lines of C-based aspell or Rust-based typos.

For example, for typos, there's an official crate-ci/typos action for easy set-up and a binary you can download locally for use in Makefile. Both work with a .typos.toml configuration file that allows custom dictionaries and file patterns. The aspell option is similar (though I'm not aware of an official GH action, it should not be too difficult to configure one...)

@Jooho
Copy link
Contributor Author

Jooho commented Jul 18, 2025

Sorry for the late reply — I was out this week.
@elevran No problem at all using a different CLI. This time I went with typos.
Hopefully this one's the right fit!

elevran
elevran previously approved these changes Jul 21, 2025
Copy link
Collaborator

@elevran elevran left a comment

Choose a reason for hiding this comment

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

minor changes left.

@elevran
Copy link
Collaborator

elevran commented Jul 21, 2025

@Jooho would you kindly fix this spelling error as part of the PR?

$ ./typos .
Warning: "Schedulling" should be "Scheduling".
error: `Schedulling` should be `Scheduling`
  --> ./docs/create_new_filter.md:118:22
    |
118 |  we leave the `types.SchedullingContext` parameter unnamed. Filters
    |                      ^^^^^^^^^^^
    |
Error: Process completed with exit code 2.

@elevran
Copy link
Collaborator

elevran commented Jul 28, 2025

@Jooho -
Thanks for the PR. It's very close to completion.
Just checking up to see if you can find time to complete this or need any assistance?
Please let us know.

@Jooho
Copy link
Contributor Author

Jooho commented Jul 29, 2025

I fixed existing typos too.

elevran
elevran previously approved these changes Jul 29, 2025
elevran
elevran previously approved these changes Jul 29, 2025
@elevran elevran enabled auto-merge (squash) July 29, 2025 15:38
@elevran elevran disabled auto-merge July 29, 2025 15:40
elevran
elevran previously approved these changes Jul 29, 2025
- fixed typos
- added gitaction for typos

Signed-off-by: Jooho Lee <jlee@redhat.com>
@elevran elevran merged commit c220900 into llm-d:main Jul 29, 2025
3 checks passed
@Jooho Jooho deleted the code_spell branch July 29, 2025 18:33
pierDipi pushed a commit to pierDipi/llm-d-inference-scheduler that referenced this pull request Nov 28, 2025
Signed-off-by: konflux-internal-p02 <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
Co-authored-by: konflux-internal-p02[bot] <170854209+konflux-internal-p02[bot]@users.noreply.github.com>
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.

Explore options for spell checking in GH actions on PRs

4 participants