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 a CI spell check job for changed files on PRs #1255

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jochem-brouwer
Copy link
Member

@jochem-brouwer jochem-brouwer commented Jan 20, 2025

This PR adds a CI job which will run a cspell check on only the changed files (as compared to master).

As a proof of concept and to show how spell fails (and false positives) should be fixed I've included two files.

image

Some notes:

  • The "bonus" we get when we merge this PR is that typo-PRs will thus now also get ran through the cspell check 😄 👍
  • The check runs in case sensitivity, for instance adding "Beiko" to known words will flag "beiko"
  • I've added area-specific words to the global word list, like EIP, Holesky, etc.
  • To add known words to a markdown file add a "comment" using [//] # cSpell:words WORD_LIST, see: https://stackoverflow.com/a/20885980
  • In the Attendees section of the Meeting, wrap this in (names are obv. flagged as "spell error"):
### Attendees
[//] # cSpell:disable
* <Attendee name 1>
* <Attendee name 2>
...
* <Attendee name N>
[//] # cSpell:enable

Note: cspell defaults to US English which is why "standardisation" is flagged on Meeting-202.

Obviously open for discussion, but I think for now a spell check job on new files would be handy. On meeting-202 it found one obvious error: eaps -> EIPs. Have not checked the other files (besides throwing them in the cspell job, this obviously flags a lot of errors)

@jochem-brouwer
Copy link
Member Author

Hmm ok I had assumed that some warning would pop up that PR-triggered actions are not triggered here, but it seems not. The action does run on my forked repo though: jochem-brouwer#2

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.

1 participant