-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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.
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
src: remove empty name check in node_env_var.cc #36133
src: remove empty name check in node_env_var.cc #36133
Conversation
/CC @addaleax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once the still-relevant part of the comment is back :)
@@ -175,10 +175,7 @@ Local<Array> RealEnvStore::Enumerate(Isolate* isolate) const { | |||
int env_v_index = 0; | |||
for (int i = 0; i < count; i++) { | |||
#ifdef _WIN32 | |||
// If the key starts with '=' it is a hidden environment variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m not sure why this line is being removed, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I'll add that back in. Thanks for reviewing. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
The empty name check has been removed since this has landed: libuv/libuv#2473
1206c69
to
76ae744
Compare
Commit Queue failed- Loading data for nodejs/node/pull/36133 ✔ Done loading data for nodejs/node/pull/36133 ----------------------------------- PR info ------------------------------------ Title src: remove empty name check in node_env_var.cc (#36133) Author Darshan Sen (@RaisinTen) Branch RaisinTen:update/empty-name-check-node_env_var.cc -> nodejs:master Labels C++, author ready Commits 1 - src: remove empty name check in node_env_var.cc Committers 1 - raisinten PR-URL: https://github.com/nodejs/node/pull/36133 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/36133 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott -------------------------------------------------------------------------------- ✖ Last GitHub CI failed ℹ Last Full PR CI on 2020-12-13T14:47:03Z: https://ci.nodejs.org/job/node-test-pull-request/34927/ - Querying data for job/node-test-pull-request/34927/ ✔ Build data downloaded ✔ Last Jenkins CI successful ℹ This PR was created on Mon, 16 Nov 2020 12:54:51 GMT ✔ Approvals: 2 ✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/36133#pullrequestreview-550926817 ✔ - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36133#pullrequestreview-552392117 -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/423272881 |
Landed in 6837a6d...5a637e9 |
The empty name check has been removed since this has landed: libuv/libuv#2473 PR-URL: #36133 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The empty name check has been removed since this has landed: libuv/libuv#2473 PR-URL: #36133 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The empty name check has been removed since this has landed: libuv/libuv#2473 PR-URL: #36133 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The empty name check has been removed since this has landed:
libuv/libuv#2473
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes