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

src: fix unused private field warning by cjihrig · Pull Request #28036 · nodejs/node · GitHub

/ node Public

src: fix unused private field warning - #28036

Closed
cjihrig wants to merge 1 commit into
nodejs:masterfrom
cjihrig:unused
Closed

src: fix unused private field warning#28036
cjihrig wants to merge 1 commit into
nodejs:masterfrom
cjihrig:unused

Conversation

cjihrig commented Jun 3, 2019

Copy link
Copy Markdown
Contributor

This fixes the following warning (and subsequent warnings that occurred from fixing the original warning):

../src/inspector/runtime_agent.h:26:16: warning: private field 'env_' is not used [-Wunused-private-field]
  Environment* env_;
               ^
1 warning generated.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Copy link
Copy Markdown
Collaborator

ZYSzys added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. inspector Issues and PRs related to the V8 inspector protocol labels Jun 3, 2019

nodejs-github-bot commented Jun 3, 2019
edited by cjihrig
Loading

Copy link
Copy Markdown
Collaborator

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

EDIT(cjihrig): CI was yellow.

cjihrig changed the title src: fix unused private field error src: fix unused private field warning Jun 3, 2019

sam-github 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. Is the explicit related to the _env fix?

cjihrig commented Jun 3, 2019

Copy link
Copy Markdown
Contributor Author

Is the explicit related to the _env fix?

Yes. After removing the unused constructor argument, you get the following warning: Zero-parameter constructors should not be marked explicit.

danbev commented Jun 4, 2019

Copy link
Copy Markdown
Contributor

Could we fast track this?

If you agree please give a 👍 and if you disagree please remove the fast-track label.

danbev added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 4, 2019

danbev commented Jun 5, 2019

Copy link
Copy Markdown
Contributor

Landed in 2c76874.

danbev closed this Jun 5, 2019
danbev pushed a commit that referenced this pull request Jun 5, 2019
PR-URL: #28036
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
cjihrig deleted the unused branch June 5, 2019 21:01
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
PR-URL: #28036
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR mentioned this pull request Jun 17, 2019
codebytere added a commit to electron/node that referenced this pull request Jul 3, 2019
Removed in nodejs/node#28036; this patch can
be removed when we update to v12.5.0
codebytere added a commit to electron/node that referenced this pull request Jul 3, 2019
Removed in nodejs/node#28036; this patch can
be removed when we update to v12.5.0
MarshallOfSound pushed a commit to electron/node that referenced this pull request Jul 4, 2019
Removed in nodejs/node#28036; this patch can
be removed when we update to v12.5.0
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fast-track PRs that do not need to wait for 48 hours to land. inspector Issues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL