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 env_ field from env.h #25784

Closed
wants to merge 1 commit into from
Closed

src: remove unused env_ field from env.h #25784

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jan 29, 2019

Currently the following compiler warnings is generated:

In file included from ../src/env-inl.h:28:
../src/env.h:521:18:
warning: private field 'env_' is not used [-Wunused-private-field]
    Environment* env_;
                 ^
1 warning generated.

This commit removes this unused field.

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

@danbev
src: removed unused env_ field from env.h
f08b251 
Currently the following compiler warnings is generated:
In file included from ../src/env-inl.h:28:
../src/env.h:521:18:
warning: private field 'env_' is not used [-Wunused-private-field]
    Environment* env_;
                 ^
1 warning generated.

This commit removes this unused field.
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jan 29, 2019
@danbev
Copy link
Contributor Author

danbev commented Jan 29, 2019

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

@danbev
Copy link
Contributor Author

danbev commented Jan 29, 2019 •
edited
Loading

Re-run of failing node-test-commit-linux (✔️)

@danbev danbev added the fast-track PRs that do not need to wait for 48 hours to land. label Jan 29, 2019
@danbev
Copy link
Contributor Author

danbev commented Jan 29, 2019

Please give a 👍 if you think this can be fast-tracked, or if you disagree please remove the fast-track label.

@refack
Copy link
Contributor

refack commented Jan 30, 2019 •
edited
Loading

Resume: https://ci.nodejs.org/job/node-test-pull-request/20433/
https://ci.nodejs.org/job/node-test-pull-request/20434/

@danbev danbev changed the title src: removed unused env_ field from env.h src: remove unused env_ field from env.h Jan 30, 2019
danbev added a commit that referenced this pull request Jan 30, 2019
@danbev
src: remove unused env_ field from env.h
dd682ce 
Currently the following compiler warnings is generated:
In file included from ../src/env-inl.h:28:
../src/env.h:521:18:
warning: private field 'env_' is not used [-Wunused-private-field]
    Environment* env_;
                 ^
1 warning generated.

This commit removes this unused field.

PR-URL: #25784
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented Jan 30, 2019

Landed in dd682ce.

@refack Thanks for running CI! It was run and then I only re-ran the failing one, so I think this PR is fully covered by CI even though the UI was/is marked as red.

@danbev danbev closed this Jan 30, 2019
@danbev danbev deleted the env_unused_var branch January 30, 2019 04:25
targos pushed a commit that referenced this pull request Jan 30, 2019
@danbev @targos
src: remove unused env_ field from env.h
1d7c963 
Currently the following compiler warnings is generated:
In file included from ../src/env-inl.h:28:
../src/env.h:521:18:
warning: private field 'env_' is not used [-Wunused-private-field]
    Environment* env_;
                 ^
1 warning generated.

This commit removes this unused field.

PR-URL: #25784
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@danbev @nodejs-github-bot @refack @jasnell @addaleax @joyeecheung @richardlau