-
Notifications
You must be signed in to change notification settings - Fork 3.5k
chore: from mdspell to codespell #15149 #15167
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
base: main
Are you sure you want to change the base?
Conversation
a1e55d9 to
256d281
Compare
Related argoproj#15149 mdspell is a bit too strict let's explore an alternative. I see codespell has a good amount of starts and it looks maintained. Signed-off-by: Gianluca Arbezzano <[email protected]>
256d281 to
d591ed9
Compare
Joibel
left a comment
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.
Whilst I wouldn't mind using codespell for the code, it's not really suitable for checking documentation, it's not a spell checker in the traditional sense.
I quite liked the idea of using vale, but it might be difficult or too draconian on style until we'd updated all the docs to match kubernetes style. I don't have a correct answer here.
We'd also need to delete the .spelling file if we merged this PR.
| $(TOOL_SNIPDOC) run | ||
|
|
||
| $(TOOL_MDSPELL): Makefile | ||
| # update this in Nix when upgrading it here |
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.
Should also remove these lines.
| npm list -g [email protected] > /dev/null || npm i -g [email protected] | ||
| endif | ||
|
|
||
| $(TOOL_CODESPELL): Makefile |
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.
No need for duplicate lines here, just put the dependencies on the same line as the target.
Makes sense, I didn't expect this to be the right approach, but it is a way to start the conversation. The issue targets Or I can add the words that mdspell complains about into the .spelling files so it will start to work again for it. Then I will add mdspell check for the various features make targets similar to what I did for the #15165 ) I think the last solution is the easiest and does not require new tooling. What do you prefer? |
Ok, lets spell check the individual features and we can switch to a better spelling tool another time then. I'd prefer fixes to the feature files over lots and lots of new words in .spelling |
Related #15149
Motivation
mdspell is a bit too strict let's explore an alternative. I see
codespell has a good amount of starts and it looks maintained.
Modifications
Replace mdsell with codespell in Makefile
Verification
CI should be happy