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

feat: add configuration for typos-rs-npm and update formatting scripts #1994

Closed
wants to merge 4 commits into from

Conversation

IanWorley
Copy link

I have add the spell checker as you requested. I would like to know would you like me to expose the Github_Token to the workflows? What other configurations would like done to the typos?

#1943

@brillout
Copy link
Member

brillout commented Dec 2, 2024

Neat ✨

I would like to know would you like me to expose the Github_Token to the workflows?

What GitHub token? What is it used for?

What other configurations would like done to the typos?

Can we set --files ./docs/* inside _typos.toml instead?

I think we should add **/dist/ to the ignore list.

Instead of adding it to the format script let's create two new npm scripts spellcheck and spellcheck:check, and let's create a new CI job for it. It's nice to see in the CI overview what exactly is failing.

@IanWorley IanWorley force-pushed the ci/add-spell-checker branch from 3b5e7b2 to bcd5101 Compare December 3, 2024 21:00
@IanWorley
Copy link
Author

@brillout My reasoning to expose the github_token was to reduce the odds of being ratelimt which could disrupt the github action workflow when trying to download typos package from github. I do not think it is possible to select the directory for typos to spell check. There is only ways to exclude files in _typos.toml file. And in my current changes I have added a new github action for typos

@brillout
Copy link
Member

brillout commented Dec 3, 2024

My reasoning to expose the github_token was to reduce the odds of being ratelimt which could disrupt the github action workflow when trying to download typos package from github.

I see. Yes, I guess we can add it publicly for now and add it privately if it's an issue to have it public. I guess we can always generate a new one?

The PR LGTM. The only thing is that the CI seems to be failing, ideas why? It seems like it doesn't work on windows?

@IanWorley
Copy link
Author

@brillout As of right now I do not know why pnpm is stuck on windows.

@brillout
Copy link
Member

brillout commented Dec 3, 2024

Since we have "typos-rs-npm": "github:dalisoft/typos-rs-npm" in devDependencies, couldn't we do typos-rs-npm --files ./docs/* instead of pnpx github:dalisoft/typos-rs-npm --files ./docs/*?

@brillout
Copy link
Member

brillout commented Dec 3, 2024

@brillout As of right now I do not know why pnpm is stuck on windows.

I don't have a windows machine to try it out. Feel free to push experimental commits in this PR to see what causes the CI to hang.

@brillout
Copy link
Member

@brillout brillout closed this Dec 28, 2024
@brillout
Copy link
Member

Let's re-open (hopefully soon) after one of these two resolves.

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.

2 participants