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.

Sign up for GitHub

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

Jump to bottom

Add Spell Checker to CI #668

Merged
merged 15 commits into from
Apr 4, 2024
Merged

Add Spell Checker to CI #668

merged 15 commits into from
Apr 4, 2024

Conversation

h4shk4t
Copy link
Contributor

@h4shk4t h4shk4t commented Feb 1, 2024

Which problem is this PR solving?

Description of the changes

  • Simply added a new cspell job to the existing ci-test workflow.
  • Will perform the spell check in the content directory and its subdirectories.
  • Added the same set of dictionary as suggested in the reference PR in the issue thread. (Let me know if any more words need to be added)

@netlify Netlify
Copy link

netlify bot commented Feb 1, 2024 •
edited
Loading

✅ Deploy Preview for romantic-neumann-1959d7 ready!

Name Link
🔨 Latest commit 64e4bd4
🔍 Latest deploy log https://app.netlify.com/sites/romantic-neumann-1959d7/deploys/660f1d001d0b7d0008674460
😎 Deploy Preview https://deploy-preview-668--romantic-neumann-1959d7.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.

@h4shk4t
Add spell checker to test workflow (jaegertracing#665)
13b9a6a 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@yurishkuro
Merge branch 'main' into spellchecker
f9f0b86 
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
yurishkuro
yurishkuro reviewed Feb 2, 2024
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

doesn't look like it did anything in the CI run

.project-words.txt Outdated Show resolved Hide resolved
@h4shk4t
Add spellchecker script
896c90f 
Add spellchecker script

Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>

Remove from test action

Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>

Add dictionary

Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>

Cleanup

Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
Switch to aspell
58fa352 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
yurishkuro
yurishkuro reviewed Feb 7, 2024
View reviewed changes
my_dictionary.txt Outdated Show resolved Hide resolved
yurishkuro
yurishkuro reviewed Feb 7, 2024
View reviewed changes
my_dictionary.txt Outdated Show resolved Hide resolved
@yurishkuro
Copy link
Member

The spellchecker flags normal words, so I don't see it as a viable solution at this point. Maybe it needs better dictionaries. Also, I would argue it should not be checking test formatted as code, because that will always bring a lot of noise.

@h4shk4t
Copy link
Contributor Author

h4shk4t commented Mar 15, 2024

@yurishkuro I did some digging and found the latest dictionary by aspell from 2020 in one of their mirrors. It shows some improvement where it is not flagging words like mentorship, analytics and more (verified this manually). However there are still a lot more common words which are still being flagged like roadmap and onboarding. I was trying Hunspell again, but I am facing issues with making it ignore hexadecimal values (will try to look for some bypasses to fix this issue in the meantime). Kindly advise if I should continue with aspell with a custom dictionary which also includes words like roadmap and onboarding, or to try and make Hunspell reduce false positives from code blocks and hexadecimal values.

@yurishkuro
Copy link
Member

@h4shk4t I don't have preference in the tool, I only have preference for the end result - low friction :-)

@h4shk4t
Add cspell spellchecker
28ca36c 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
Delete old spellcheckers
d163420 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
Copy link
Contributor Author

h4shk4t commented Mar 17, 2024

@yurishkuro Made the changes. Using cspell binary for spellchecking. Gives a promising accuracy and far lesser tolerance for false positives. Kindly let me know if this suffices.

Best Regards

yurishkuro
yurishkuro reviewed Mar 17, 2024
View reviewed changes
project-words.txt Outdated Show resolved Hide resolved
yurishkuro
yurishkuro reviewed Mar 17, 2024
View reviewed changes
cspell.json Outdated Show resolved Hide resolved
@h4shk4t
Fixes
0338dee 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
yurishkuro
yurishkuro reviewed Mar 17, 2024
View reviewed changes
Makefile Show resolved Hide resolved
@h4shk4t
Revert ci-test.yml
3c63ddc 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
fix
ffd95e2 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
Call spellcheck from ci-test.yml
38b3f3c 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@yurishkuro
Copy link
Member

The lint step is catching some real misspellings now, plus a few words that need to go to the dictionary

@h4shk4t
Copy link
Contributor Author

h4shk4t commented Mar 21, 2024

The lint step is catching some real misspellings now, plus a few words that need to go to the dictionary

Yes, I will add the new words to the dictionary (must have missed them earlier). What should I do about the misspellings?

@h4shk4t
Add new words to project-words.txt
5841486 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
@h4shk4t
Spelling fixes across all documents
f86cbca 
Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com>
yurishkuro and others added 3 commits April 4, 2024 17:20
@yurishkuro
Merge branch 'main' into spellchecker
556861d 
Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
@yurishkuro
fix remaining words
f4a2a2c 
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro
missing fixes
64e4bd4 
Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro yurishkuro merged commit 916ed3a into jaegertracing:main Apr 4, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yurishkuro yurishkuro yurishkuro approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add spell checker to CI
2 participants
@h4shk4t @yurishkuro