Skip to content
This repository has been archived by the owner. It is now read-only.

Set loglevel to notice when installing npm packages #36

Merged
merged 1 commit into from Mar 10, 2015

Conversation

@Starefossen
Copy link
Member

@Starefossen Starefossen commented Mar 8, 2015

This PR sets --loglevel to notice when installing npm packages in the ONBUILD image.

Rationale:

Packages breaks from time to time. By default npm has --loglevel set to warning when installing, and if the install fails for some reason the entire log is written to npm-debug.log. If you have not mounted /usr/src/app as a volume this file is now inside of the container which just exited.

In order to view the log you will have to copy the file from the docker container onto you host using the docker cp command. In my opinion this creates unnecessary overhead, and may not be entirely intuitive to novices.

@pesho
Copy link
Member

@pesho pesho commented Mar 10, 2015

LGTM, thanks 👍

pesho added a commit that referenced this pull request Mar 10, 2015
Set loglevel to notice when installing npm packages
@pesho pesho merged commit a72f08a into nodejs:master Mar 10, 2015
@pesho
Copy link
Member

@pesho pesho commented Mar 10, 2015

@Starefossen according to docker-library/official-images#543 (comment) the same can be achieved using an environment variable. Do you think we should:

(a) just revert this change and leave it to the user to set env NPM_CONFIG_LOGLEVEL info if needed

(b) set env NPM_CONFIG_LOGLEVEL info by default in all image variants (in such case can you please prepare a new PR)

Starefossen pushed a commit to Starefossen/docker-iojs that referenced this pull request Mar 10, 2015
This reverts commit a72f08a, reversing
changes made to 79e00fb.
@Starefossen
Copy link
Member Author

@Starefossen Starefossen commented Mar 10, 2015

I have submitted a new PR which reverts this one and adds NPM_CONFIG_LOGLEVEL to the appropriate Dockerfiles.

@Starefossen Starefossen deleted the Starefossen:patch-2 branch Mar 10, 2015
Starefossen pushed a commit to Starefossen/docker-node that referenced this pull request Mar 4, 2016
Close: nodejs#57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@dnt.no>
Starefossen pushed a commit to Starefossen/docker-node that referenced this pull request Jun 24, 2016
Hans Kristian Flaatten
Close: nodejs#57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@dnt.no>
Starefossen pushed a commit to Starefossen/docker-node that referenced this pull request Jun 24, 2016
Hans Kristian Flaatten
Close: nodejs#57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@dnt.no>
Starefossen pushed a commit to Starefossen/docker-node that referenced this pull request Jun 24, 2016
Hans Kristian Flaatten
Close: nodejs#57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@dnt.no>
Starefossen pushed a commit to Starefossen/docker-node that referenced this pull request Jun 24, 2016
Hans Kristian Flaatten
Close: nodejs#57
Related: nodejs/docker-iojs#36

Signed-off-by: Hans Kristian Flaatten <hans.kristian.flaatten@dnt.no>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.