Check for misspellings in CI #1825
Conversation
github-actions
bot
added
the
tools and infrastructure
github-actions
bot
added
documentation
|
I'm impressed by how few typos our process has let through so far! My thought is that spelling checks are really helpful, but I'd prefer them as a comment in a PR, and not as a blocker that requires overrides in order to pass. That provides authors the info they need when they need it, but is a much lower long term cost. |
|
I see.
|
|
Hi @szepeviktor, thanks for opening this PR. I'm still giving some thought about how to fit in spell checks like this, with some other linting on the table. I'm not exactly sure if/when I'll be able to land this, but I do want to get your feature identifier fix into the release this week. I've opened #1918 and credited you with the |
|
@ddbeck Hello 👋🏻 I could rebase this PR every week and report typos to you.
|
|
@ddbeck new findings: "glpyhs" |
|
new findings: "signficantly" |
szepeviktor
force-pushed
the
patch-1
branch
2 times, most recently
from
918e7d0 to
5939764
Compare
|
@ddbeck Typos live with us. |
szepeviktor
force-pushed
the
patch-1
branch
2 times, most recently
from
eed5973 to
be8dfc6
Compare
There was a problem hiding this comment.
Hi @szepeviktor, thanks again for opening this PR.
This PR came up at the WebDX CG meeting today. I'm reviewing this now, but I've also asked that others have a look too. I'm not familiar with this tool, so I don't have a ton of confidence in my review of it—you'll find that I'm mostly asking questions at this point.
| RELEASE_ASSET_URL="$( | ||
| gh api /repos/crate-ci/typos/releases/latest \ | ||
| --jq '."assets"[] | select(."name" | test("^typos-.+-x86_64-unknown-linux-musl\\.tar\\.gz$")) | ."browser_download_url"' | ||
| )" |
There was a problem hiding this comment.
I have a question here: ci/typos seems to be available as a GitHub Action (which I imagine we could pin and upgrade with Dependabot). How does this approach compare to using the Action?
Also, have you seen this used in other open projects with a lot of churn on files? I'd be curious to know what a real-world PR looks like following a workflow like this (I'm curious how contributors respond to the feedback given).
There was a problem hiding this comment.
I do not recommend running typos in "passive" mode as a typo in code could be detrimental.
E.g. Go language encourages programmers to use 2 and 3 letter variable names. They usually look like English misspellings. That is all I know.
There was a problem hiding this comment.
typos is just a super useful software - I need an LLM to argue.
Please love typos.
| git grep --files-with-matches --null -I -e '.' \ | ||
| | xargs --null --verbose -- "${{ runner.temp }}/typos/typos" --format json \ | ||
| | jq --raw-output '"::warning file=\(.path),line=\(.line_num),col=\(.byte_offset)::\"\(.typo)\" should be \"" + (.corrections // [] | join("\" or \"") + "\".")' \ | ||
| || true |
There was a problem hiding this comment.
This is rather complicated! Is there any way this can be broken up or commented to make each step better explained? Or maybe an example showing the transformation going on here? I'm just not sure what this is doing.
There was a problem hiding this comment.
Actually it is enumerate files -> run typos -> convert output to GitHub check annotations.
All this code is hiding inside the GitHub Action of typos.
We talked about that typos should not stop a PR that is why I've copied the code from the Action and modified errors to warnings.
I've sent around ~500 PR-s advertising typos: https://github.com/pulls?q=is%3Apr+author%3Aszepeviktor+archived%3Afalse+Fix+typos
~5% of the people hate it.
This PR is explosive 💥
and includes file renaming