util: doc deprecate legacy inspect signature by refack · Pull Request #22753 · nodejs/node

refack · 2018-09-07T18:51:57Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-09-07T18:52:22Z

@refack build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/825/pipeline

targos · 2018-09-07T18:54:24Z

version: REPLACEME

refack · 2018-09-07T18:56:00Z

Since this is deprecating an undocumented feature, I recommend to treat this as semver-patch. As such this needs @nodejs/tsc approval (see some previous discussion in #22751)

targos · 2018-09-07T19:04:22Z

The collaborator guide says:

Documentation-Only Deprecations may be handled as semver-minor or semver-major changes.

Let's make this semver-minor and it won't need special TSC approval (It will also be highlighted in the release notes, which is a good thing).

targos · 2018-09-07T19:11:05Z

you can already add pr-url: https://github.com/nodejs/node/pull/22753

vsemozhetbyt · 2018-09-07T19:24:44Z

Nit: node -> Node.js.

I removed it completely (since it could be assumed that Node.js is the object on the sentence)

Refs: nodejs#22751

Trott · 2018-09-07T20:05:34Z

    example below. **Default:** `true`.
 * Returns: {string} The representation of passed object

+Deprecation notice: The legacy function signature that was changed in v0.9.3


Nit: remove Deprecation notice:.

I'd suggest simplifying this greatly:

The legacy function signature `util.inspect(object, [showHidden], [depth], [colors])` is deprecated.

One problem with this is that it can be interpreted as saying that util.inspect(object) is deprecated but it is not. Can we indicate that the showHidden, depth, and colors arguments are deprecated instead of pointing to the function signature?

jasnell · 2018-09-07T20:42:15Z

This would need to include a deprecation code assignment in deprecations.md

addaleax

Why do we deprecate this?

addaleax · 2018-09-14T17:06:29Z

 added: v0.3.0
 changes:
+  - version: REPLACEME
+    pr-rul: https://github.com/nodejs/node/pull/22753


addaleax · 2018-09-14T17:07:11Z

 * Returns: {string} The representation of passed object

+Deprecation notice: The legacy function signature that was changed in v0.9.3
+`util.inspect(object, [showHidden], [depth], [colors])` has been deprecated.


I’d put this closer to the end of the description… this is probably not relevant to a lot of people (and if it is, we shouldn’t deprecate it)

trivikr · 2018-10-01T15:06:28Z

Ping

refack · 2018-10-01T18:43:38Z

Why do we deprecate this?

IMHO we should either deprecate or document.
Personally I was pro deprecation, and having a monomorphic signature, but there was pushback.

Ref: #23205

nodejs-github-bot added doc Issues and PRs related to Node.js documentation. util Issues and PRs related to the built-in util module. labels Sep 7, 2018

refack mentioned this pull request Sep 7, 2018

util: inspect - remove very old legacy args #22751

Closed

1 of 4 tasks

targos reviewed Sep 7, 2018

View reviewed changes

Comment thread doc/api/util.md Outdated

targos Sep 7, 2018

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

version: REPLACEME

targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 7, 2018

targos reviewed Sep 7, 2018

View reviewed changes

Comment thread doc/api/util.md Outdated

targos Sep 7, 2018

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

you can already add pr-url: https://github.com/nodejs/node/pull/22753

vsemozhetbyt reviewed Sep 7, 2018

View reviewed changes

refack force-pushed the doc-deprecate-inspect-legacy branch from 5c52da1 to 76a64c3 Compare September 7, 2018 19:25

util: doc deprecate legacy inspect signature

cfd0067

Refs: nodejs#22751

refack force-pushed the doc-deprecate-inspect-legacy branch from 76a64c3 to cfd0067 Compare September 7, 2018 19:26

Trott reviewed Sep 7, 2018

View reviewed changes

refack mentioned this pull request Sep 13, 2018

ci-change: timeout in post-build-status nodejs/build#1494

Merged

addaleax reviewed Sep 14, 2018

View reviewed changes

refack added the blocked PRs that are blocked by other issues or PRs. label Oct 1, 2018

refack closed this Oct 1, 2018

refack deleted the doc-deprecate-inspect-legacy branch October 1, 2018 18:48

refack mentioned this pull request Oct 2, 2018

doc: document "flat" util.inspect signature #23205

Closed

Conversation

refack commented Sep 7, 2018

Checklist

Uh oh!

nodejs-github-bot commented Sep 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack commented Sep 7, 2018

Uh oh!

targos commented Sep 7, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasnell commented Sep 7, 2018

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

trivikr commented Oct 1, 2018

Uh oh!

refack commented Oct 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

refack commented Oct 1, 2018 •

edited

Loading