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

src: remove unused variable in node_file #18200

Closed
wants to merge 1 commit into from
Closed

src: remove unused variable in node_file #18200

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jan 17, 2018

Currently the following warning is reported when building:

../src/node_file.cc:542:18:
warning: unused variable 'context' [-Wunused-variable]
  Local<Context> context = env->context();
                 ^
../src/node_file.cc:566:18:
warning: unused variable 'context' [-Wunused-variable]
  Local<Context> context = env->context();
                 ^
2 warnings generated.

The commit removes the unused variables.

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

src

@danbev
src: remove unused variable in node_file
d099874 
Currently the following warning is reported when building:

../src/node_file.cc:542:18:
warning: unused variable 'context' [-Wunused-variable]
  Local<Context> context = env->context();
                 ^
../src/node_file.cc:566:18:
warning: unused variable 'context' [-Wunused-variable]
  Local<Context> context = env->context();
                 ^
2 warnings generated.

The commit removes the unused variables.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. labels Jan 17, 2018
@danbev
Copy link
Contributor Author

danbev commented Jan 17, 2018

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

joyeecheung
joyeecheung approved these changes Jan 17, 2018
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

Oops, I think I forgot to remove those in the stats migration PR, sorry...and thanks!

@jasnell
Copy link
Member

jasnell commented Jan 17, 2018 •
edited
Loading

Duplicate of #18205 btw ... or I should say #18205 is a duplicate of this one ;)

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 17, 2018
@BridgeAR
Copy link
Member

@danbev uh, I just landed the other PR first. I am sorry that I missed that this came up first.

I hope it is fine that I close this PR.

@BridgeAR BridgeAR closed this Jan 19, 2018
@danbev danbev deleted the node_file_unused_var branch February 28, 2018 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@tniessen tniessen tniessen approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@richardlau richardlau richardlau 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. c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@danbev @jasnell @BridgeAR @cjihrig @tniessen @joyeecheung @richardlau @nodejs-github-bot