Skip to content

tools: capitalize more comments - #26483

Closed
BridgeAR wants to merge 3 commits into
nodejs:masterfrom
BridgeAR:capitalize-more-comments
Closed

tools: capitalize more comments#26483
BridgeAR wants to merge 3 commits into
nodejs:masterfrom
BridgeAR:capitalize-more-comments

Conversation

@BridgeAR

@BridgeARBridgeAR commented Mar 7, 2019

Copy link
Copy Markdown
Member

Stricten the lint rule to validate all comments from 30 characters on.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@BridgeAR
benchmark,doc,lib,test: capitalize comments
caefa19
@BridgeAR
tools: update capitalized-comments rule
3832edd
This strictens to rule to validate comments from 30 characters on.
@nodejs-github-bot

nodejs-github-bot commented Mar 7, 2019

Copy link
Copy Markdown
Collaborator

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2792/pipeline

@nodejs-github-botnodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 7, 2019
@addaleax

addaleax commented Mar 7, 2019

Copy link
Copy Markdown
Member

Can we also maybe punctuate the comments in the same go? We don’t lint this for C++, but it’s been a common review comment.

@BridgeAR

BridgeAR commented Mar 7, 2019

Copy link
Copy Markdown
MemberAuthor

@addaleax I would like to do that but not manually and currently I can only think about writing a new eslint rule to automate it properly. If anyone has a suggestion to automate it in a simple fashion, I am happy to do that.

vsemozhetbyt
vsemozhetbyt approved these changes Mar 7, 2019
View reviewed changes

@vsemozhetbytvsemozhetbyt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With some false-positive and consistency nits.

Comment threadbenchmark/net/tcp-raw-s2c.js Outdated
Comment threaddoc/api/dgram.md Outdated
Comment threaddoc/api/dgram.md Outdated
Comment threaddoc/api/errors.md Outdated
Comment threaddoc/api/stream.md Outdated
Comment threadtest/parallel/test-stream3-cork-end.js Outdated
Comment threadtest/parallel/test-stream3-cork-end.js Outdated
Comment threadtest/parallel/test-stream3-cork-uncork.js Outdated
Comment threadtest/parallel/test-stream3-cork-uncork.js Outdated
Comment threadtest/pummel/test-stream-pipe-multi.js Outdated
@BridgeAR

BridgeAR commented Mar 8, 2019

Copy link
Copy Markdown
MemberAuthor

@vsemozhetbyt I addressed your comments and also added a couple of dots while going through your comments.

@BridgeAR

BridgeAR commented Mar 9, 2019

Copy link
Copy Markdown
MemberAuthor

Lite CI https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/2839/

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 9, 2019
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 9, 2019
@BridgeAR
benchmark,doc,lib,test: capitalize comments
f8763bb
PR-URL: nodejs#26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Mar 9, 2019
@BridgeAR
tools: update capitalized-comments rule
3414bc7
This strictens to rule to validate comments from 30 characters on.
PR-URL: nodejs#26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BridgeAR

BridgeAR commented Mar 9, 2019

Copy link
Copy Markdown
MemberAuthor

Landed in f8763bb and 3414bc7

@BridgeARBridgeAR closed this Mar 9, 2019
BridgeAR added a commit that referenced this pull request Mar 13, 2019
@BridgeAR
benchmark,doc,lib,test: capitalize comments
2a1cc66
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit that referenced this pull request Mar 13, 2019
@BridgeAR
tools: update capitalized-comments rule
836a29a
This strictens to rule to validate comments from 30 characters on.
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit that referenced this pull request Mar 13, 2019
@BridgeAR
benchmark,doc,lib,test: capitalize comments
2b57c65
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit that referenced this pull request Mar 13, 2019
@BridgeAR
tools: update capitalized-comments rule
06c5b84
This strictens to rule to validate comments from 30 characters on.
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BridgeARBridgeAR mentioned this pull request Mar 13, 2019
Merged
BridgeAR added a commit that referenced this pull request Mar 14, 2019
@BridgeAR
benchmark,doc,lib,test: capitalize comments
52a0d76
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BridgeAR added a commit that referenced this pull request Mar 14, 2019
@BridgeAR
tools: update capitalized-comments rule
3e2e779
This strictens to rule to validate comments from 30 characters on.
PR-URL: #26483
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@BridgeAR
BridgeAR deleted the capitalize-more-comments branch January 20, 2020 11:55
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targostargostargos approved these changes
@bnoordhuisbnoordhuisAwaiting requested review from bnoordhuis
@TrottTrottAwaiting requested review from Trott
+1 more reviewer
@vsemozhetbytvsemozhetbytvsemozhetbyt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.lib / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@BridgeAR@nodejs-github-bot@addaleax@targos@vsemozhetbyt