-
Notifications
You must be signed in to change notification settings - Fork 39
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
add spell check for news and readme #1021
Conversation
similar to flux-framework/flux-core#5074 I'll try to fix up a ton of the typos in here too. There's a lot more false positives than in flux-core, so will have to go through it a little bit one by one :-( Don't have to necessarily fix all of them, just a good chunk. |
Sounds good - thanks @chu11 ! Please push / edit this branch as you see fit. |
@vsoch similar the fixup in flux-core, could the commit be split into a typo fix commit and a CI fix. Thanks. |
All set. |
i think you forgot to tweak title of the |
I didn’t forget, I thought it was OK. I think perfection of commit messages isn’t the best use of time, but since you asked I will change it! The first I think was OK because when I ran the check I did check news and the README, but it didn’t find any issues in the README. |
I can't seem to add on top of the PR, wondering if i'm not counted as a "maintainer" on flux-sched. My commits can be yoinked from: |
I think @vsoch has to add you as collaborator to researchapps... |
Done! I invited all of you. |
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.
LGTM!
resource/traversers/dfu_impl.cpp
Outdated
@@ -293,7 +293,7 @@ const std::vector<Resource> &dfu_impl_t::test (vtx_t u, | |||
* For this case, when you visit the "node" resource vertex, the next | |||
* Jobspec resource that should be used at the next recursion should | |||
* be socket[1]. And we enable this if pristine is true. | |||
* But then once the first match is made, any mis-mismatch afterwards | |||
* But then once the first match is made, any miss-mismatch afterwards |
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.
I don't think I understand this either way...
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.
The typo software stumbled here, seems like it should be "mismatch"
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.
doh, i'll fix this up
Problem: the NEWS.md has spelling errors, and goes out to releases. Solution: fix spelling errors in NEWS.md Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Problem: we should automate spell checking Solution: add spell check to CI Signed-off-by: vsoch <vsoch@users.noreply.github.com>
Problem: The function not_feasable() was misspelled. Rename it to not_feasible() and update callers.
fixed up that one remaining typo, force pushed it, hope that's ok! setting MWP |
Codecov Report
@@ Coverage Diff @@
## master #1021 +/- ##
========================================
- Coverage 74.3% 74.3% -0.1%
========================================
Files 86 86
Lines 9434 9434
========================================
- Hits 7018 7017 -1
- Misses 2416 2417 +1
|
Problem: the NEWS.md is not spell checked, and goes out to releases.
Solution: add a strong spell checker to be run on CI so we catch issues.