Skip to content

report: list envvars using uv_os_environ()#28963

Merged
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:envvars
Aug 11, 2019
Merged

report: list envvars using uv_os_environ()#28963
cjihrig merged 1 commit into
nodejs:masterfrom
cjihrig:envvars

Conversation

@cjihrig
Copy link
Copy Markdown
Contributor

@cjihrig cjihrig 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
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Aug 5, 2019
@cjihrig cjihrig added the blocked PRs that are blocked by other issues or PRs. label Aug 5, 2019
Copy link
Copy Markdown
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment thread src/node_report.cc Outdated
@cjihrig cjihrig force-pushed the envvars branch 3 times, most recently from 8b89159 to 097a154 Compare August 11, 2019 15:35
@cjihrig cjihrig added report Issues and PRs related to process.report. and removed blocked PRs that are blocked by other issues or PRs. lib / src Issues and PRs related to general changes in the lib or src directory. labels Aug 11, 2019
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

nodejs-github-bot commented Aug 11, 2019

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>
@cjihrig cjihrig merged commit 9b221e5 into nodejs:master Aug 11, 2019
@cjihrig cjihrig deleted the envvars branch August 11, 2019 18:21
targos pushed a commit that referenced this pull request Aug 19, 2019
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

report Issues and PRs related to process.report.

5 participants