-
Notifications
You must be signed in to change notification settings - Fork 48
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
Spelling #79
Spelling #79
Conversation
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
Signed-off-by: Josh Soref <[email protected]>
# Prefix for configfuration files (multipath.conf) | ||
# Prefix for configuration files (multipath.conf) |
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.
This PR is because a peer asked me about this...
@@ -187,7 +187,7 @@ The default is: \fBno\fR | |||
. | |||
.TP | |||
.B multipath_dir | |||
(Deprecated) This option is not supported any more, and the value is ignored. | |||
(Deprecated) This option is not supported anymore, and the value is ignored. |
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.
It's controversial whether "any more" is wrong, even with the meaning "no longer". The Cambridge dictionary says both spellings are possible.
Whatever.
I am evaluating adding your spell checking action to our workflows, so that PRs like this can be avoided in the future. Thanks! |
@jsoref, this goes far beyond my knowledge about GitHub actions. I have added your file to my "tip" branch but I am getting 2453 unrecognized words. You apparently didn't need to add all these words ... what am I doing wrong? |
@mwilck: cherry-pick jsoref@d03eaeb Or, to answer your question: I had to configure away ~700 of the words (by excluding files, and masking parts of lines with patterns) and then add ~1800 of them. The good thing is that adding additional items to the expect file is fairly trivial, so as you go it isn't a big deal -- the workflow helps you maintain that file (and as well as excludes, and to a lesser extent patterns and the workflow itself). |
Yes, there are a couple of boilerplate patterns that aren't truly necessary, and candidates is mostly a bootstrapping thing, although it means that if a project gains new stuff that fits a pattern it can be automatically suggested. (Note that you accidentally tagged someone else in #79 (comment)) If you're going to do this, you should probably replace |
Thanks. I'll wait for the other maintainers' thoughts. |
I'm pretty ambivalent about this. I'm sure it will catch spelling mistakes. But I assume it will mostly just catch things that need to get added to the expect file, which at least appears easy to update. I don't see any of the boilerplate patterns hurting anything. We could always just limit spell-checking to things like the man pages, README.md, and the public library .h files. This would cut down on the busy work but still make sure the stuff visible to non-multipath-developers is spell-checked. |
You can use |
I won't have time to work on the GH integration short-term. I will put it on a long-term todo list. Thanks to Josh's efforts, quite a few speling errors have been eliminated for now ;-) |
@jsoref, I've been playing around with this a bit, strongly restricting the set of files scanned for misspellings like @bmarzins suggested. I found that check-spelling recommends adding words like the following:
These words originate from groff escape sequences in man pages, like |
Yep, you can copy this pair of patterns: If you have examples beyond that set, I'm happy to adjust the thing -- certainly the patterns there have done a fairly reasonable job in my testing. |
Another one
Here check-spelling sees the word |
Right candidate-patterns should be cover that as well |
Do I have to copy the entire |
No.
Yes. candidate.patterns is just a thing that will automatically suggest things, you can just copy the items you want into And yes, candidates has at least one hex pattern (probably the hex/color one). |
With openSUSE@391767f, I eventually got 🟢 for this workflow 😁 Patterns that might be interesting for you:
|
Thanks, added as check-spelling/spell-check-this@ee04617 Note that I tend to rewrite commits in --- I'll definitely be adjusting the WWNN/WWPN patterns -- probably just merging them into a single pattern as check-spelling uses the comment before a pattern when it suggests the pattern to hint to readers about its purpose. |
This is a follow-up to #37
Fixes misspellings identified by the check-spelling action.
The misspellings have been reported at https://github.com/jsoref/multipath-tools/actions/runs/7405969189/attempts/1#summary-20149678198
The action will report that the changes in this PR would make it happy: https://github.com/jsoref/multipath-tools/actions/runs/7405969286/attempts/1#summary-20149678465