Skip to content

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
# 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
Contributor

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

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.

9 participants