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: spellingscontrole #59

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: spellingscontrole #59

wants to merge 7 commits into from

Conversation

nicorikken
Copy link
Member

Start van de spellingscontrole opzet op basis van CSpell.
Automatisering d.m.v. GitHub Actions moet nog.

Start voor issue #6

Signed-off-by: Nico Rikken [email protected]

Start van de spellingscontrole opzet op basis van CSpell.
Automatisering d.m.v. GitHub Actions moet nog.

Start voor issue #6

Signed-off-by: Nico Rikken <[email protected]>
@nicorikken nicorikken force-pushed the feature/spellcheck branch 5 times, most recently from 99c178f to 8225655 Compare March 28, 2023 06:04
GitHub Actions workflow for the CSpell spellcheck. Action from
https://github.com/streetsidesoftware/cspell-action

Additional dictionary has to be installed seperately upfront, as described in
streetsidesoftware/cspell-action#181

Signed-off-by: Nico Rikken <[email protected]>
@marcvanandel
Copy link
Member

Nice! Ik wist niet dat dit bestond! En nu zouden de woorden die niet herkend worden, ergens moeten worden toegevoegd? Op zich wel mooi ... of leidt dit tot (te veel) overhead? 🤔

@nicorikken
Copy link
Member Author

nicorikken commented Mar 28, 2023

Ja, CSpell stelt je in staat een custom woordenlijst te definiëren: https://cspell.org/docs/dictionaries-custom/ dus dat kunnen we gebruiken. Maar wel de vraag hoe je om gaat met fouten die blijven betaan.

Ja dus risico op overhead, maar tegelijkertijd ook een extra controle.

Gelicenseerd is de meer gangbare spelling.
Toegestane worden om de spellingscontrole te laten slagen.

Signed-off-by: Nico Rikken <[email protected]>
@nicorikken nicorikken marked this pull request as ready for review March 28, 2023 12:46
Copy link
Member

@marcvanandel marcvanandel left a comment

Choose a reason for hiding this comment

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

Van global naar local libs?

README.md Show resolved Hide resolved
cspell.config.yaml Outdated Show resolved Hide resolved
steps:
- uses: actions/checkout@v3
- name: Install dictionaries
run: npm install @cspell/dict-nl-nl
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... Ik heb nu yarn gebruikt ... 🤔

Copy link
Member

@marcvanandel marcvanandel Sep 14, 2023

Choose a reason for hiding this comment

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

Sowieso zou dit npm ci moeten zijn ipv npm install ... maar nog beter yarn gebruiken met een lock file ... Mee eens, @nicorikken ?

See https://github.com/marketplace/actions/npm-or-yarn-install-with-caching

@MauriceHendriks
Copy link
Member

Zou die yarn.lock niet onderdeel moeten zijn van de .gitignore?

@marcvanandel
Copy link
Member

Zou die yarn.lock niet onderdeel moeten zijn van de .gitignore?

Nee, juist niet, @MauriceHendriks , want daarin zijn alle versies 'vast gezet'. Zo kun je lokaal testen en werkt dat overal hetzelfde. Er is apart onderhoud op alle dependencies nodig ... wat in GitHub ook goed wordt ondersteund vanuit 'Dependabot' of 'Renovate'.

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.

3 participants