-
Notifications
You must be signed in to change notification settings - Fork 12
CI: Add automated spellchecking #47
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
Conversation
#### Problem There are often typos in code or READMEs, but they don't get caught. #### Summary of changes Integrate `cargo spellcheck` to automate spellchecking all Rust code in the repo. I wasn't sure where to put the additional configuration, so I hope `scripts` was an ok destination.
lorisleiva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just a few nits.
.github/workflows/main.yml
Outdated
| - name: Install cargo-spellcheck | ||
| uses: taiki-e/install-action@v2 | ||
| with: | ||
| tool: cargo-spellcheck | ||
|
|
||
| - name: Setup Environment | ||
| uses: ./.github/actions/setup | ||
| with: | ||
| cargo-cache-key: cargo-spellcheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: as a convention, the other jobs are using the setup action immediately after the git checkout step. Could we do the same here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, good call, I misunderstood how the cache action works, thanks!
package.json
Outdated
| "clients:rust:test": "zx ./scripts/client/test-rust.mjs", | ||
| "template:upgrade": "zx ./scripts/upgrade-template.mjs" | ||
| "template:upgrade": "zx ./scripts/upgrade-template.mjs", | ||
| "spellcheck:rust": "cargo spellcheck --code 1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we call this rust:spellcheck in preparation for febo's work on unifying scripts?
@febo After your refactoring, I think this should be part of the lint subtasks, so it would probably need to be located in scripts/rust/lint-spellcheck. Lmk what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!
febo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Problem
There are often typos in code or READMEs, but they don't get caught.
Summary of changes
Integrate
cargo spellcheckto automate spellchecking all Rust code in the repo.I wasn't sure where to put the additional configuration, so I hope
scriptswas an ok destination.