-
-
Notifications
You must be signed in to change notification settings - Fork 170
ci: add spellcheck with "typos" #687
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
Nice, seems we had a lot of typos :D |
|
||
[default.extend-identifiers] | ||
# FOOBAR = "FOOBAR" | ||
|
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.
Are these default values? If so, can we drop the config file entirely?
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.
It might happen that we have to add something there eventually. This is also necessary in a few other projects of mine. I thought, it might be easier for us if we already have the file as template in the repo. What do 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.
I would slightly prefer that we drop the config file until such time as we need it, just to avoid having unnecessary stuff in the root of the repo. It's not something I feel super strongly about though.
.github/workflows/qa.yml
Outdated
- uses: cachix/install-nix-action@v19 | ||
with: | ||
nix_path: nixpkgs=channel:nixos-22.11-small | ||
- run: nix-shell -p typos --run "typos ." |
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.
https://github.com/crate-ci/typos/blob/master/docs/github-action.md suggests using their own action, seems like it could be a little simpler than using the nix action.
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.
Oh, yes. The nix solution is complete overkill when there is typo action :D - good catch!
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.
hmm, it seems like the typos action doesn't work.. :(
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.
The latest release was 10 minutes ago: https://github.com/crate-ci/typos/releases/tag/v1.13.20 Maybe it just needs a little time for release pipelines to finish?
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.
Do we want this instability or use the nix approach instead? In nix, it is guaranteed to be stable 😋
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 there's a note in the doc: "for any of the examples above, make sure that you choose a release or commit as a version, and not a branch (which is a moving target)."
So probably instead of uses: crate-ci/typos@master
it should be uses: crate-ci/[email protected]
.
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, you are right. btw, it works: https://github.com/rust-osdev/uefi-rs/actions/runs/4376220833/jobs/7658009793
42f9778
to
6824d4f
Compare
This adds the "typos" utility as additional CI step. Additionally, all existing typos are fixed.
This adds the "typos" utility as additional CI step. Additionally, all existing typos are fixed.
Checklist