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 CSpell checker to CI and fix typos #3590

Merged
merged 23 commits into from
Aug 17, 2024
Merged

Add CSpell checker to CI and fix typos #3590

merged 23 commits into from
Aug 17, 2024

Conversation

jochem-brouwer
Copy link
Member

@jochem-brouwer jochem-brouwer commented Aug 14, 2024

This PR adds cspell to the CI. To use it locally, from the repository root, use npm run cspell. This will spell-check all *.ts files in the packages. It will also check the markdown, but it will not check the markdowns in the docs.

The output of cspell will give context and - if possible - also give word suggestions how to fix it.

If a word is clearly correct but cspell flags it, add it to the words list in config/cspell-md.json (for markdown) or config/cspell-ts.json. There are two jsons, one for scripts and one for the md files.

If a word is intentionally incorrect, but it should be ignored, add // cspell:disable-line to a script or <!--- cspell:disable-line ---> to a markdown file.

The CI will trigger the npm run cspell job, if it finds any mistakes, the CI will throw.

  • Add and fix markdown
  • Add to CI
  • Test CI fails if cspell throws

** To review this **:

Please check the config/cspell-md.json and the config/cspell-ts.json for any words which I have marked as correct, but they are still wrong. Some words might need context on why these are added, to check: remove the file from the json, save the file, and then run npm run cspell:ts for scripts or npm run cspell:md for markdown to see the context in which this word is found.

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

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

Really important: please do not merge this yet respectively do not run the execution part (the doc changes), otherwise integrating the release PR #3527 changes into master will be a total merge hell!!

(I mentioned this kind of problem set a couple of times in the chat, actually I am getting a bit tired to always need to play the last guard resort to look for these kind of things 😳)

Apart from this base complaint this is really a great PR though! 😄

@jochem-brouwer jochem-brouwer marked this pull request as draft August 15, 2024 10:14
@jochem-brouwer
Copy link
Member Author

I think I got a little bit too enthusiastic, because I was aware of this (this is also the reason why I have split up the md/ts files and have not committed any typedoc changes). Have marked PR as draft so it also cannot be accidentally merged.

@holgerd77
Copy link
Member

Haha, then maybe just mention it and mark as "blocked" to reduce my 😱 moments! 😂

@jochem-brouwer jochem-brouwer marked this pull request as ready for review August 16, 2024 11:04
@jochem-brouwer
Copy link
Member Author

Once we merge this, I suggest the spell check job is added to the required CI jobs.

config/README.md Outdated Show resolved Hide resolved
@holgerd77
Copy link
Member

I didn't know we had THAT many spelling mistakes in our docs 🤯 (so many POAP opportunities lost 😂 ).

Copy link
Contributor

@scorbajio scorbajio left a comment

Choose a reason for hiding this comment

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

Really useful addition to the CI 🙂

packages/block/test/from-rpc.spec.ts Outdated Show resolved Hide resolved
@jochem-brouwer
Copy link
Member Author

@scorbajio See #3599, I did those renames there :) I think I will actually merge that one into this PR before moving it into master (after I've finished the CI conflicts)

* remove almost all cspell:ignore

* more spell changes

* cspell: fix problems

* evm: fix quadCoefficient

* cspell: fixes

* remove disable line
@jochem-brouwer
Copy link
Member Author

Hi all, I have addressed concerns in #3599 and just squash-merged it in this commit: 5167780

I think this will resolve most/all concerns/issues?

@jochem-brouwer jochem-brouwer dismissed holgerd77’s stale review August 17, 2024 13:17

PR mentioned got merged

@jochem-brouwer jochem-brouwer requested review from holgerd77 and removed request for holgerd77 August 17, 2024 13:18
@jochem-brouwer jochem-brouwer merged commit 4a8761a into master Aug 17, 2024
35 checks passed
@jochem-brouwer jochem-brouwer deleted the cspell branch August 17, 2024 18:16
@jochem-brouwer
Copy link
Member Author

Thanks for all the reviews 👍 😄

holgerd77 pushed a commit that referenced this pull request Aug 19, 2024
* monorepo: add cspell, add ALL unknown words to valid words

* cspell: split unknown words in ts/md

* filter out wrong words in cspell-ts.json

* cspell ignore hex values

* fix typos in all packages

* cspell: use cache

* cspell: update commands

* cspell: update md/ts words

* Typo fixes for README/CHANGELOG files

* cspell: ensure all relevant monorepo md files are checked

* ci: add cspell job

* cspell: update command

* temp add bogus to markdown

* remove bogus spell

* update ci name

* fix remaining typos + add words to cspell dict

* Update packages/client/CHANGELOG.md

* Update packages/util/CHANGELOG.md

* address review

* Remove almost all `cspell:ignore` (#3599)

* remove almost all cspell:ignore

* more spell changes

* cspell: fix problems

* evm: fix quadCoefficient

* cspell: fixes

* remove disable line

---------

Co-authored-by: Gabriel Rocheleau <[email protected]>
@holgerd77
Copy link
Member

Looking at this once again, and this is really monumental! 🤯

One of the big structural updates for the whole monorepo! 💯 🎉

holgerd77 added a commit that referenced this pull request Aug 19, 2024
…Code put() Fix, VerkleSM out, runTx()+Code Opts) (#3601)

* Use shallowCopy() Caches copy() optimization also for VerkleSM to avoid Caches bundling on non-Caches default

* Fix default state manager missing direct code write when used without caches

* Do not initialize caches for default VM state manager

* Remove copy test not making sense any more under generalized cache/no-cache conditions

* Some more solid/qualified EVM dummy blockchain + interface naming to allow for exporting

* Use EVMMockBlockchain(Interface) as default for the VM, adjust some tests

* Move @ethereumjs/blockchain to dev dependencies in VM

* Rebuild package-lock.json

* Adjust/fix some client tests

* Lint fix

* Add Verkle SM methods as optional methods to interface, replace VerkleSM imports and castings in VM

* Also align client (no real effect yet, but generally try to work more on the interfaces and not the classes directly)

* Initialize runTx() default block with simpler constructor to avoid drawing all txs in

* Fully switch to DEFAULT_HEADER in VM.runTx() to avoid drawing in block code

* Opcode list size optimization

* More optimizations

* Precompile code optimizations

* More optimizations (precompile index.ts file)

* Some more

* Some doc compatification

* Add CSpell checker to CI and fix typos (#3590)

* monorepo: add cspell, add ALL unknown words to valid words

* cspell: split unknown words in ts/md

* filter out wrong words in cspell-ts.json

* cspell ignore hex values

* fix typos in all packages

* cspell: use cache

* cspell: update commands

* cspell: update md/ts words

* Typo fixes for README/CHANGELOG files

* cspell: ensure all relevant monorepo md files are checked

* ci: add cspell job

* cspell: update command

* temp add bogus to markdown

* remove bogus spell

* update ci name

* fix remaining typos + add words to cspell dict

* Update packages/client/CHANGELOG.md

* Update packages/util/CHANGELOG.md

* address review

* Remove almost all `cspell:ignore` (#3599)

* remove almost all cspell:ignore

* more spell changes

* cspell: fix problems

* evm: fix quadCoefficient

* cspell: fixes

* remove disable line

---------

Co-authored-by: Gabriel Rocheleau <[email protected]>

* Fix spell check

* Remove accidentally committed examples/test.ts file

---------

Co-authored-by: Jochem Brouwer <[email protected]>
Co-authored-by: Gabriel Rocheleau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants