-
Notifications
You must be signed in to change notification settings - Fork 163
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
Fixed typo in scripts/cspell/project-words.txt #753
Conversation
✅ Deploy Preview for romantic-neumann-1959d7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: Priyansh Sao <[email protected]>
f58e5fa
to
7f25149
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
scripts/cspell/project-words.txt
Outdated
ingesters | ||
ingester's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert order change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the change is reverted and the make spellcheck command is run, the output displays a sorting error -
cat scripts/cspell/project-names.txt | grep -v '^#' | grep -v '^\s*$' | tr ' ' '\n' > scripts/cspell/project-names-parsed.txt
cd scripts/cspell && ./spellcheck.sh
sort: project-words.txt:40: disorder: ingesters
project-words.txt is not sorted.
make: *** [Makefile:60: spellcheck] Error 1
It is failing now |
So, should I consider reverting the change? |
You need to make sure the linter passes. You can test it locally. |
The reason ingester's appears before ingesters in a sorted list is that it is treated as "ingester" followed by an apostrophe, whereas ingesters is treated as "ingester" followed by the letter s. This means that the apostrophe has a higher priority in the sort order than the letter s. If you were to place ingester's after ingesters, it would trigger a sorting error because the list is expected to follow alphabetical order, taking into account all characters, including punctuation. This is why it fails to pass the linter. |
I don't understand the motivation for your explanation - what are you trying to prove? Before your change the linter was not complaining, now it does complain. |
To check i ran the 'make checkspell' in main branch and it showed the same error , i understand what you are saying but in main branch where nothing is changed still it is showing the error. |
Signed-off-by: Yuri Shkuro <[email protected]>
@yurishkuro, I apologize for the confusion earlier. There was an issue with my terminal, which caused the error to appear. |
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist