-
Notifications
You must be signed in to change notification settings - Fork 23
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
lint: update a name that wasn't capitalized #340
Conversation
I've found some docs that don't capitalize the first letter of Valgrind (namely the contributor doc related to investigating native memory leaks). This additional lint should help to fix that grammatical issue.
Thanks, @nschonni! Co-authored-by: Nick Schonning <nschonni@gmail.com>
You'll need a corresponding PR in the other repo to clean it up, before the CI here will go green |
Already on it! |
Finished this a while ago but forgot to link the PR |
index.js
Outdated
@@ -102,6 +102,7 @@ const plugins = [ | |||
{ no: "[Rr][Ff][Cc]\\d+", yes: "RFC <number>" }, | |||
{ yes: "Unix" }, | |||
{ yes: "V8" }, | |||
{ no: "valgrind", yes: "Valgrind" }, |
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 for the PR. Two comments:
- We don't need the
no
entry in this case. Just theyes
is enough (and will prevent people from doing other things likeValGrind
orVALGRIND
). - The entries are in alphabetical order. Can you move this one up one line (so it's after
Unix
and beforeV8
)?
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.
Can do, and will do! :)
- Update index.js to remove unnecessary `no` clause - Move it to alphabetical order
Whoops, I should have squashed rather than rebased. Going to force push to fix that. |
Thanks for the contribution! 🎉 |
I've found some docs that don't capitalize the first letter of Valgrind (namely the contributor doc related to investigating native memory leaks). This additional lint should help to fix that grammatical issue.