FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

n-api: fix compiler warning by cjihrig · Pull Request #21597 · nodejs/node · GitHub

/ node Public

n-api: fix compiler warning - #21597

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:warn
Jul 2, 2018
ConversationCommits (1)ChecksFiles changed
Merged

n-api: fix compiler warning#21597
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:warn

Conversation

cjihrig commented Jun 29, 2018

Copy link
Copy Markdown
Contributor

private field 'async_context' is not used [-Wunused-private-field]

Refs: #17887

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

nodejs-github-bot commented Jun 29, 2018

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels Jun 29, 2018

addaleax commented Jun 30, 2018

Copy link
Copy Markdown
Member

LGTM; but: @gabrielschulhof I thought you added async context tracking? I’m not sure where that’s being propagated here…

gabrielschulhof commented Jun 30, 2018

Copy link
Copy Markdown
Contributor

@addaleax I believe I simply forgot to remove that field when class TsFn started inheriting from AsyncResource and therefore did no longer need to store its async_context.

danbev commented Jul 2, 2018

Copy link
Copy Markdown
Contributor

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

danbev commented Jul 2, 2018

Copy link
Copy Markdown
Contributor
node-test-commit-windows-fanned failure looks unrelated

console output:

09:30:47 not ok 566 sequential/test-inspector-break-when-eval
09:30:47   ---
09:30:47   duration_ms: 0.403
09:30:47   severity: fail
09:30:47   exitcode: 1
09:30:47   stack: |-
09:30:47     [test] Connecting to a child Node process
09:30:47     [test] Testing /json/list
09:30:47     [err] Debugger listening on ws://127.0.0.1:61103/56a876e9-c99e-4083-961c-f8ff452a4e8d
09:30:47     [err] For help, see: https://nodejs.org/en/docs/inspector
09:30:47     [err] 
09:30:47     [out] Ready!
09:30:47     [out] 
09:30:47     [test] Setting up a debugger
09:30:47     [test] Breaking in the code
09:30:47     [test] Step over console statement and test output
09:30:47     [out] 0 3
09:30:47     [out] 
09:30:47     [test] Verify node waits for the frontend to disconnect
09:30:47     [err] Debugger attached.
09:30:47     [err] Waiting for the debugger to disconnect...
09:30:47     [err] 
09:30:47     { AssertionError [ERR_ASSERTION]: Input A expected to strictly equal input B:
09:30:47     + expected - actual
09:30:47     
09:30:47     - 0
09:30:47     + 3221225477
09:30:47         at runTests (c:\workspace\node-test-binary-windows\test\sequential\test-inspector-break-when-eval.js:65:10)
09:30:47         at process._tickCallback (internal/process/next_tick.js:68:7)
09:30:47       generatedMessage: true,
09:30:47       name: 'AssertionError [ERR_ASSERTION]',
09:30:47       code: 'ERR_ASSERTION',
09:30:47       actual: 0,
09:30:47       expected: 3221225477,
09:30:47       operator: 'strictEqual' }
09:30:47     1
09:30:47   ...

n-api: fix compiler warning
a27b0a3 
private field 'async_context' is not used [-Wunused-private-field]

PR-URL: nodejs#21597
Refs: nodejs#17887
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
cjihrig merged commit a27b0a3 into nodejs:master Jul 2, 2018
cjihrig deleted the warn branch July 2, 2018 15:14
targos pushed a commit that referenced this pull request Jul 3, 2018
n-api: fix compiler warning
c870372 
private field 'async_context' is not used [-Wunused-private-field]

PR-URL: #21597
Refs: #17887
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
targos mentioned this pull request Jul 3, 2018
Merged
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Dec 28, 2018
n-api: fix compiler warning
b285f1d 
private field 'async_context' is not used [-Wunused-private-field]

PR-URL: nodejs#21597
Refs: nodejs#17887
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 18, 2019
n-api: fix compiler warning
ca8ba15 
private field 'async_context' is not used [-Wunused-private-field]

PR-URL: #21597
Refs: #17887
Backport-PR-URL: #25002
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
n-api: fix compiler warning
c5a11dc 
private field 'async_context' is not used [-Wunused-private-field]

PR-URL: #21597
Refs: #17887
Backport-PR-URL: #25002
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins mentioned this pull request Mar 26, 2019
Merged
snyk-bot mentioned this pull request Mar 18, 2020
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

gabrielschulhof gabrielschulhof approved these changes
+1 more reviewer
danbev danbev approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL