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

Add typos - Source code spell checker #3565

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

ameknite
Copy link
Contributor

@ameknite ameknite commented Mar 4, 2024

Same as #3557 but with typos

Advantages:

  • Fast to run on monorepos
  • Low false positives so you can run on PRs

For reference typos is used in:
Bevy - bevyengine/bevy#12036
Wgpu - gfx-rs/wgpu#5191

@notgull
Copy link
Member

notgull commented Mar 5, 2024

Looks like the current implementation takes around 8 seconds. So it's faster and it seems more thorough as well. So I'm ready to switch to this instead of cspell.

@rust-windowing/winit Thoughts?

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

I've used typos a lot by now and I had a great experience!
I don't have anything against cspell and I also don't mind running multiple spellcheckers as well.

Except the two nits, LGTM.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Should be good after review addressed.

@kchibisov kchibisov mentioned this pull request Mar 5, 2024
5 tasks
@ameknite
Copy link
Contributor Author

ameknite commented Mar 5, 2024

This fixes the same typos as #3556. Do I removed them from here and wait for the other pr to be merged, or I keep them?

@kchibisov
Copy link
Member

This fixes the same typos as #3556. Do I removed them from here and wait for the other pr to be merged, or I keep them?

No, will merge your patch instead fixing them.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Could you squash your changes?

@ameknite
Copy link
Contributor Author

ameknite commented Mar 5, 2024

squashed

Copy link
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

LGTM except the small nit.

typos.toml Outdated Show resolved Hide resolved
@ameknite ameknite force-pushed the add-typos branch 2 times, most recently from 0a9e598 to 6b4c99e Compare March 6, 2024 00:31
Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Looks good to me. Once this is rebased I will merge.

Given that typos are frequent and may appear in the public API spell
check code on CI.
@kchibisov
Copy link
Member

@ameknite I've rebased things myself.

@kchibisov kchibisov merged commit b2f9fad into rust-windowing:master Mar 7, 2024
52 checks passed
notgull added a commit to forkgull/glutin that referenced this pull request Mar 12, 2024
As per the precedent set in winit, adds a spellchecker using the typos
crate to CI.

cc rust-windowing/winit#3565

Signed-off-by: John Nunley <[email protected]>
kchibisov pushed a commit to rust-windowing/glutin that referenced this pull request Mar 13, 2024
As per the precedent set in winit, adds a spellchecker using the typos
crate to CI.

Links: rust-windowing/winit#3565
Signed-off-by: John Nunley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants