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

Revert log level to npm's default level #535

Merged
merged 1 commit into from
Oct 18, 2017
Merged

Revert log level to npm's default level #535

merged 1 commit into from
Oct 18, 2017

Conversation

LaurentGoderre
Copy link
Member

Fixes #528

nschonni

This comment was marked as off-topic.

Sign in to view

chorrell

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre LaurentGoderre force-pushed the loglevel branch 4 times, most recently from ba86e96 to a3fb16f Compare October 3, 2017 17:28
@LaurentGoderre
Copy link
Member Author

@chorrell done! Sorry about that noise.

chorrell

This comment was marked as off-topic.

Sign in to view

@chorrell
Copy link
Contributor

chorrell commented Oct 3, 2017

Now worries. Looks good now!

SimenB

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre
Copy link
Member Author

@SimenB @chorrell can we do the doc change in another PR. We need to discuss the wording etc.

@chorrell
Copy link
Contributor

chorrell commented Oct 5, 2017

Yeah, a follow-up PR for doc seems fine to me.

@LaurentGoderre
Copy link
Member Author

Rebase done!

SimenB

This comment was marked as off-topic.

Sign in to view

@chorrell
Copy link
Contributor

chorrell commented Oct 5, 2017

Well even if we land this, the images in the hub still have the npm logging noted in the current docs. I guess the docs will need to note that newer images will have default logging and older ones don’t.

I’m fine either way.

LaurentGoderre added a commit that referenced this pull request Oct 18, 2017
@LaurentGoderre
Updated documentation for new log level
9542e1a 
Completes #535
@LaurentGoderre
Copy link
Member Author

@chorrell can we merge this soon? Otherwise I will keep having to rebase

@chorrell
Copy link
Contributor

Yes, but we still need an update for the docs to note that newer images will have default logging and older ones don’t. That update can either be in this PR or a new PR.

@LaurentGoderre LaurentGoderre mentioned this pull request Oct 18, 2017
Merged
@LaurentGoderre
Copy link
Member Author

@chorrell already done! #550

@chorrell
Copy link
Contributor

Perfect. Once Travis CI is green I'll merge and then do #550

@LaurentGoderre
Copy link
Member Author

We can discuss the docs wording further too

@chorrell chorrell merged commit c1f363a into nodejs:master Oct 18, 2017
@LaurentGoderre LaurentGoderre deleted the loglevel branch October 18, 2017 17:31
LaurentGoderre added a commit that referenced this pull request Oct 20, 2017
@LaurentGoderre
Updated documentation for new log level
89e388f 
Completes #535
LaurentGoderre added a commit to LaurentGoderre/docker-node that referenced this pull request Oct 23, 2017
@LaurentGoderre
Updated documentation for new log level
6c84971 
Completes nodejs#535
SimenB pushed a commit that referenced this pull request Oct 24, 2017
@LaurentGoderre @SimenB
Updated documentation for new log level (#550)
5acf858 
Completes #535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nschonni nschonni nschonni left review comments

@SimenB SimenB SimenB approved these changes

@chorrell chorrell chorrell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LaurentGoderre @chorrell @nschonni @SimenB