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

doc: doc-only deprecation for util.log() #6161

Closed
wants to merge 1 commit into from

Conversation

JacksonTian
Copy link
Contributor

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

util

Description of change

There are more powerful loggers in user land like debug, soft
deprecate it.

see #4497 .

@@ -551,6 +551,8 @@ util.isUndefined(null)

## util.log(string)

Stability: 0 - Deprecated
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps include some alternative recommendation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we recommend 3rd package?

Copy link
Member

Choose a reason for hiding this comment

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

Recommending a specific third party package would be problematic, but saying something like, "Use a third party userland module instead" would be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok.

There are more powerful loggers in user land like `debug`, soft
deprecate it.
@JacksonTian
Copy link
Contributor Author

@jasnell I updated it.

@@ -551,6 +551,8 @@ util.isUndefined(null)

## util.log(string)

Stability: 0 - Deprecated: Use a third party userland module instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

userland is maybe redundant here? Maybe also just point to .debuglog()? Even though there are no timestamps.

Copy link
Contributor

Choose a reason for hiding this comment

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

debuglog() is quite different. See below:

Output with timestamp on stdout.

@benjamingr
Copy link
Member

Deprecating a method on a stable module should be semver major and ctc agenda.

+1 for deprecating it though.

@benjamingr benjamingr added doc Issues and PRs related to the documentations. semver-major PRs that contain breaking changes and should be released in the next major version. labels Apr 12, 2016
@mscdex mscdex added the util Issues and PRs related to the built-in util module. label Apr 12, 2016
@jasnell
Copy link
Member

jasnell commented Apr 19, 2016

This LGTM but adding it to the CTC agenda for a quick review

@jasnell
Copy link
Member

jasnell commented Apr 19, 2016

@nodejs/ctc

@mscdex
Copy link
Contributor

mscdex commented Apr 19, 2016

LGTM

@@ -49,7 +49,7 @@ where `3245` is the process id. If it is not run with that
environment variable set, then it will not print anything.

You may separate multiple `NODE_DEBUG` environment variables with a
comma. For example, `NODE_DEBUG=fs,net,tls`.
comma. For example, `NODE_DEBUG=fs,net,tls`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated change and it is possibly accepted by many.

@evanlucas
Copy link
Contributor

LGTM with @thefourtheye nit addressed

@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

@nodejs/ctc discussed this and decided to move forward with it. If there are no further objections by monday I will land before v6 is cut. There may need to be some additional tweaks to the actual text in the docs.

@jasnell jasnell added this to the 6.0.0 milestone Apr 22, 2016
@jasnell
Copy link
Member

jasnell commented Apr 25, 2016

LGTM. Fixing a couple of nits on landing

jasnell pushed a commit that referenced this pull request Apr 25, 2016
There are more powerful loggers in user land like `debug`, soft
deprecate it.

PR-URL: #6161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@jasnell
Copy link
Member

jasnell commented Apr 25, 2016

Landed in 236b7e8

@jasnell jasnell closed this Apr 25, 2016
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
There are more powerful loggers in user land like `debug`, soft
deprecate it.

PR-URL: nodejs#6161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
jasnell pushed a commit that referenced this pull request Apr 26, 2016
There are more powerful loggers in user land like `debug`, soft
deprecate it.

PR-URL: #6161
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants