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

Codebase spell checker #102

Merged
merged 10 commits into from
Jan 2, 2025

Conversation

barrymun
Copy link
Contributor

@barrymun barrymun commented Jan 1, 2025

Description

  • Add the cspell package
  • Fix any existing spelling errors in the codebase
  • Any words added to cspell-dict.txt will be ignored
  • Run the spell checker on commit (added to pre-commit hooks)
  • Package deps updated
  • Added new GitHub workflow pre-commit-checks.yml

Type of Change

  • ✨ New snippet
  • 🛠 Improvement to an existing snippet
  • 🐞 Bug fix
  • 📖 Documentation update
  • 🔧 Other (please describe):

Checklist

  • I have tested my code and verified it works as expected.
  • My code follows the style and contribution guidelines of this project.
  • Comments are added where necessary for clarity.
  • Documentation has been updated (if applicable).
  • There are no new warnings or errors from my changes.

Related Issues

Closes #82

Additional Context

Screenshots (Optional)

Click to view screenshots

Copy link

netlify bot commented Jan 1, 2025

Deploy Preview for quicksnip ready!

Name Link
🔨 Latest commit 7127ceb
🔍 Latest deploy log https://app.netlify.com/sites/quicksnip/deploys/67768cfa4a8c1f00080e1a14
😎 Deploy Preview https://deploy-preview-102--quicksnip.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@barrymun barrymun changed the title Feature - codebase spell check Codebase spell checker Jan 1, 2025
Copy link
Collaborator

@Mathys-Gasnier Mathys-Gasnier left a comment

Choose a reason for hiding this comment

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

Would you be able to add a CI or edit an existing one to run all linting, cspell, and all of that ?

Appart from that it looks wonderfull

@Mathys-Gasnier
Copy link
Collaborator

It also looks like your eslint config broke the CI, could you look at that ? No idea what caused it

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

It also looks like your eslint config broke the CI, could you look at that ? No idea what caused it

On main currently if npm run lint is executed there seems to be an error in useLanguages.tsx. Maybe these were merged around the same time before this file could be correctly linted?

@Mathys-Gasnier
Copy link
Collaborator

I fixed it, was due to the node version being 16 and eslint v9 supporting only node v18+

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

It also looks like your eslint config broke the CI, could you look at that ? No idea what caused it

@Mathys-Gasnier I think it may actually be that we're using an older version of node:
Screenshot 2025-01-01 at 21 00 00

Switching to node v16.20.2 causes the same error on local:
Screenshot 2025-01-01 at 21 02 29

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

I fixed it, was due to the node version being 16 and eslint v9 supporting only node v18+

You found the same issue as me, thanks 😄

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

Would you be able to add a CI or edit an existing one to run all linting, cspell, and all of that ?

Appart from that it looks wonderfull

Let me take a look at this.

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

@Mathys-Gasnier Just merged the latest changes; should the following be "contents" and not "content"?

check-snippets.yml
Screenshot 2025-01-01 at 21 08 37

@Mathys-Gasnier
Copy link
Collaborator

It should yeah, i'm going to push a commit fixing the typo

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

@Mathys-Gasnier I've added a new workflow "pre-commit-checks.yml" which seems to be working correctly. Currently it will execute on push and pull_request for any branch or PR, but feel free to changes this if it is not restrictive enough.

Copy link
Collaborator

@Mathys-Gasnier Mathys-Gasnier left a comment

Choose a reason for hiding this comment

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

I think you can keep it just on pull requests, we only really need to check this when merging a PR

@barrymun
Copy link
Contributor Author

barrymun commented Jan 1, 2025

I think you can keep it just on pull requests, we only really need to check this when merging a PR

This has been changed.

@Mathys-Gasnier
Copy link
Collaborator

Looks magnificent, let's wait on another review and we can then merge this

@Mathys-Gasnier
Copy link
Collaborator

As you fixed what @psychlone77 said, I think we are safe to merge this, great work !

@Mathys-Gasnier Mathys-Gasnier merged commit d00e21b into dostonnabotov:main Jan 2, 2025
5 checks passed
@barrymun barrymun deleted the feature/spellcheck branch January 2, 2025 14:17
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.

[Bug] - Spell check
3 participants