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

Explain the npm verbosity in the node image #113

Merged
merged 1 commit into from
Jul 29, 2016

Conversation

Starefossen
Copy link
Member

Attempts to explain the rationale behind the NPM_CONFIG_LOGLEVEL.

Close: #57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten [email protected]

@pesho
Copy link
Contributor

pesho commented Mar 4, 2016

LGTM

@chorrell
Copy link
Contributor

chorrell commented Mar 4, 2016

LGTM

@@ -53,6 +53,45 @@ $ docker run -it --rm --name my-running-script -v "$PWD":/usr/src/app -w
/usr/src/app node:4 node your-daemon-or-script.js
```

## Verbosity

By default Node.js Docker Image has NPM log verbosity set to `verbose` instead

This comment was marked as off-topic.

This comment was marked as off-topic.

@chorrell
Copy link
Contributor

chorrell commented Mar 4, 2016

One small nit, but other-wise looks good

By default Node.js Docker Image has NPM log verbosity set to `verbose` instead
of the default `warn`. This is because of the way Docker is isolated from the
host operating system and you are not guaranteed to be able to retrieve the
`npm-debug.log` file when something breaks.

This comment was marked as off-topic.

This comment was marked as off-topic.

@retrohacker
Copy link
Contributor

I think the technical sections are 🔥

I think we need to explain the rationale behind the decision in terms newcomers to both Docker and Node will understand while striking a balance with being technically correct.

@Starefossen Starefossen force-pushed the docs/npm-verbose-output branch 3 times, most recently from 31203ab to 87bf354 Compare June 24, 2016 12:14
@Starefossen
Copy link
Member Author

I have incorporated the feedback and added a section to explain the rationale behind the decision of changing the verbosity level of npm. Feel free to nitpick 😄

@chorrell
Copy link
Contributor

LGTM!

@retrohacker
Copy link
Contributor

Merging this as it's gone 5 months without objection

@retrohacker retrohacker merged commit 614e8e7 into nodejs:master Jul 29, 2016
tianon pushed a commit to docker-library/official-images that referenced this pull request May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants