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

doc: os.userInfo throwing SystemError #25724

Closed
wants to merge 1 commit into from
Closed

doc: os.userInfo throwing SystemError #25724

wants to merge 1 commit into from

Conversation

raido
Copy link
Contributor

@raido raido commented Jan 26, 2019 •
edited
Loading

os.userInfo throws an exception which was not documented in environments like:

docker run --user $(id -u): $(id -g)

Closes: #25714

Checklist

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem. labels Jan 26, 2019
@raido raido force-pushed the os-userinfo-doc branch 2 times, most recently from 4483091 to b805764 Compare January 26, 2019 15:00
Trott
Trott reviewed Jan 26, 2019
View reviewed changes
doc/api/os.md Outdated Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Jan 26, 2019

Welcome @raido and thanks for the pull request! I left some small comments. Thanks again!

sindresorhus
sindresorhus reviewed Jan 28, 2019
View reviewed changes
doc/api/os.md Outdated Show resolved Hide resolved
@raido
Copy link
Contributor Author

raido commented Feb 1, 2019

I see there are other methods in OS module that throw SystemErrors. Not sure if I should include all of them in this PR or open separate one for the rest of them?

vsemozhetbyt
vsemozhetbyt reviewed Feb 2, 2019
View reviewed changes
doc/api/os.md Outdated Show resolved Hide resolved
@raido
doc: os.userInfo throwing SystemError
71d26c4 
`os.userInfo` throws an exception which was not documented
in environments like: `docker run --user $(id -u): $(id -g)`

PR-URL: nodejs#25724
Fixes: nodejs#25714
@vsemozhetbyt
Copy link
Contributor

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/2497/

@vsemozhetbyt
Copy link
Contributor

Landed in d0bce9a
Thank you!

vsemozhetbyt pushed a commit that referenced this pull request Feb 3, 2019
@raido @vsemozhetbyt
doc: document os.userInfo() throwing SystemError
d0bce9a 
`os.userInfo()` throws an exception which was not documented
in environments like: `docker run --user $(id -u): $(id -g)`.

PR-URL: #25724
Fixes: #25714
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Feb 3, 2019
@raido @addaleax
doc: document os.userInfo() throwing SystemError
1f6acbb 
`os.userInfo()` throws an exception which was not documented
in environments like: `docker run --user $(id -u): $(id -g)`.

PR-URL: #25724
Fixes: #25714
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Feb 14, 2019
Merged
This was referenced Feb 15, 2019
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vsemozhetbyt vsemozhetbyt vsemozhetbyt left review comments

@Trott Trott Trott left review comments

@sindresorhus sindresorhus sindresorhus left review comments

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
doc Issues and PRs related to the documentations. os Issues and PRs related to the os subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

os.userInfo throws if no username in docker container
6 participants
@raido @Trott @vsemozhetbyt @sindresorhus @jasnell @nodejs-github-bot