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.

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

report: list envvars using uv_os_environ() #28963

Merged
merged 1 commit into from Aug 11, 2019

Conversation

@cjihrig
Copy link
Contributor

commented Aug 5, 2019

Note: This PR is a bit premature. It requires the libuv 1.31.0 update, which will happen later this week. Only the second commit (report: list envvars using uv_os_environ()) is relevant. libuv 1.31.0 includes a new API, uv_os_environ(), for iterating over the environment variables.

This PR simplifies the diagnostic report's code for listing environment variables by using uv_os_environ() instead of platform specific code.

cc: @nodejs/libuv

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
@saghul
saghul approved these changes Aug 5, 2019
Copy link
Member

left a comment

LGTM!

src/node_report.cc Outdated Show resolved Hide resolved

@cjihrig cjihrig force-pushed the cjihrig:envvars branch 3 times, most recently from 8b89159 to 097a154 Aug 11, 2019

@cjihrig cjihrig added report and removed blocked lib / src labels Aug 11, 2019

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link

commented Aug 11, 2019

report: list envvars using uv_os_environ()
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.

PR-URL: #28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@cjihrig cjihrig force-pushed the cjihrig:envvars branch from 097a154 to 9b221e5 Aug 11, 2019

@cjihrig cjihrig merged commit 9b221e5 into nodejs:master Aug 11, 2019

1 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
Travis CI - Branch Build Passed
Details

@cjihrig cjihrig deleted the cjihrig:envvars branch Aug 11, 2019

targos added a commit that referenced this pull request Aug 19, 2019
report: list envvars using uv_os_environ()
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.

PR-URL: #28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos added a commit that referenced this pull request Aug 19, 2019
report: list envvars using uv_os_environ()
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.

PR-URL: #28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@targos targos referenced this pull request Aug 19, 2019
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
report: list envvars using uv_os_environ()
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.

PR-URL: nodejs#28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
report: list envvars using uv_os_environ()
This commit simplifies the diagnostic report's code for listing
environment variables by using uv_os_environ() instead of
platform specific code.

PR-URL: nodejs#28963
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.