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.

Sign up for GitHub

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

Jump to bottom

test: remove unnecessary semicolon after macro #26618

Closed
wants to merge 1 commit into from
Closed

test: remove unnecessary semicolon after macro #26618

wants to merge 1 commit into from

Conversation

hashseed
Copy link
Member

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

@hashseed hashseed requested a review from addaleax March 12, 2019 20:24
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 12, 2019
@hashseed
Copy link
Member Author

In fact, I'm getting some warnings in my GN build with the latest build configs from Chromium upstream, if the semicolon is there:

../../node/test/cctest/test_linked_binding.cc:19:60: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
NODE_MODULE_LINKED(cctest_linkedbinding, InitializeBinding);

@addaleax
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/21486/

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 12, 2019
mhdawson
mhdawson approved these changes Mar 15, 2019
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@antsmartian
Copy link
Contributor

Landed in c5e619b 🎉 🎉

pull bot pushed a commit to SimenB/node that referenced this pull request Mar 16, 2019
@hashseed @antsmartian
test: remove unnecessary semicolon after macro
c5e619b 
PR-URL: nodejs#26618
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
@hashseed @targos
test: remove unnecessary semicolon after macro
647b996 
PR-URL: nodejs#26618
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request Mar 27, 2019
@hashseed @targos
test: remove unnecessary semicolon after macro
875ddcb 
PR-URL: #26618
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@targos targos mentioned this pull request Mar 27, 2019
Merged
@hashseed hashseed deleted the removesemicolon branch May 2, 2019 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@devsnek devsnek devsnek approved these changes

@richardlau richardlau richardlau approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@lpinca lpinca lpinca approved these changes

@antsmartian antsmartian antsmartian approved these changes

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.