Skip to content
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

src: remove empty comment in node_http2.h #16400

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 23, 2017

This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src, tools

This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Oct 23, 2017
@@ -3021,7 +3021,7 @@ def CheckComment(line, filename, linenum, next_line_start, error):
# If the comment contains an alphanumeric character, there
# should be a space somewhere between it and the // unless
# it's a /// or //! Doxygen comment.
if (Match(r'//[^ ]*\w', comment) and
if (Match(r'(?://[^ ]*\w)|(?:////\s*$)', comment) and
Copy link
Member

Choose a reason for hiding this comment

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

Do we maintain our own cpplint or is it updated from upstream from time to time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question, I don't know the answer I'm afraid. Hopefully someone can chime in. I'd be happy to open a PR upstream if this is the case.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we maintain our own judging by the file's history.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM then.

@danbev
Copy link
Contributor Author

danbev commented Oct 25, 2017

@danbev
Copy link
Contributor Author

danbev commented Oct 25, 2017

test/arm-fanned failures look unrelated

console output:

+ git clean -fdx
warning: failed to remove out/Release/.nfs00000000000f537d000002f9
Build step 'Execute shell' marked build as failure
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: *.tap
Did not find any matching files. Setting build result to FAILURE.
Checking ^not ok
Jenkins Text Finder: File set '*.tap' is empty
Notifying upstream projects of job completion
Finished: FAILURE

danbev added a commit to danbev/node that referenced this pull request Oct 25, 2017
This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

PR-URL: nodejs#16400
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Oct 25, 2017

Landed in 3e68e44

@danbev danbev closed this Oct 25, 2017
@danbev danbev deleted the node_http2.h_comment branch October 25, 2017 09:34
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

PR-URL: nodejs/node#16400
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 30, 2017
This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

PR-URL: #16400
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
gibfahn pushed a commit that referenced this pull request Oct 31, 2017
This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

PR-URL: #16400
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@gibfahn gibfahn mentioned this pull request Oct 31, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
This commit removes an "empty" comment in node_http2.h that I don't
think was intentional and as far as I can tell not a doxygen comment or
anything like that.

This was not picked up by the cpp linter so a suggestion has also been
added to the CheckComment function to detect these in the future.

PR-URL: nodejs/node#16400
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants