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: document deprecation of util._extend #4903

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@benjamingr
Member

benjamingr commented Jan 27, 2016

doc: document deprecation of util._extend

See the discussion here #4593 for more
details as well as the 2016-01-20 minutes.

doc: document deprecation of util._extend
doc: document deprecation of util._extend

See the discussion here #4593 for more
details as well as the 2016-01-20 minutes.
@benjamingr

This comment has been minimized.

Member

benjamingr commented Jan 27, 2016

Something went very wrong with git in #4902 , so I created a new PR.

@@ -548,6 +548,16 @@ Deprecated predecessor of `console.log`.
Deprecated predecessor of `console.log`.
## util._extend(obj)
Stability: 0 - Deprecated: Use Object.assign() instead.

This comment has been minimized.

@thefourtheye

thefourtheye Jan 27, 2016

Contributor

I suggested a change in #4907. If it gets through, we can make the Object.assign() a link.

This comment has been minimized.

@benjamingr

benjamingr Jan 28, 2016

Member

Great, now that it landed I'll edit it into it.

@jasnell

This comment has been minimized.

Member

jasnell commented Jan 27, 2016

@nodejs/ctc ... defensively marking as semver-major since it's a deprecation. If anyone feels that too strong, lemme know

@@ -548,6 +548,16 @@ Deprecated predecessor of `console.log`.
Deprecated predecessor of `console.log`.
## util._extend(obj)

This comment has been minimized.

@brendanashworth

brendanashworth Jan 28, 2016

Member

I think you need to escape (\_) the underscore for markdown.

Stability: 0 - Deprecated: Use Object.assign() instead.
`_extend` was never intended to be used outside of internal NodeJS modules. The

This comment has been minimized.

@thefourtheye

thefourtheye Jan 28, 2016

Contributor

I think we conveyed this at the top of the page. https://nodejs.org/api/util.html#util_util

@rmg rmg referenced this pull request Jan 28, 2016

Closed

util: deprecate util._extend #4593

@ljharb

This comment has been minimized.

ljharb commented Jan 28, 2016

Is there any benefit in recommending a polyfill so that modules can use Object.assign and retain compatibility with older nodes? Specifically, require('object.assign/polyfill')() will return the best available function (ie, native, unless it's broken or absent).

@benjamingr

This comment has been minimized.

Member

benjamingr commented Jan 28, 2016

@ljharb in all honesty I think most differences @domenic outlined, while extremely important in evaluating removing _extend are things "average" users would never run into. A single example using ._extend in the wild that would break can go a long way to convince me otherwise though.

@ljharb

This comment has been minimized.

ljharb commented Jan 28, 2016

Yes, I'm not trying to strengthen the case to deprecate _extend, i'm saying that if users want to use Object.assign but don't want to drop support for older nodes that don't have it, they could use the object.assign package.

@silverwind

This comment has been minimized.

Contributor

silverwind commented Feb 9, 2016

Not sure I agree with this, naming it "deprecation" seems weird. I think if we're going to do this, I'd rather have it be labeled "discouragement" or similar. Also, as others have pointed out, Object.assign is not 100% compatible. -1 from me.

@vkurchatkin

This comment has been minimized.

Member

vkurchatkin commented Feb 9, 2016

-1. We shouldn't add something that was never public just to say that it's deprecated

@benjamingr

This comment has been minimized.

Member

benjamingr commented Feb 9, 2016

@vkurchatkin @silverwind I was under the impression I'm following the consensus reached at the TSC 2016-01-20 meeting.

@silverwind

This comment has been minimized.

Contributor

silverwind commented Feb 9, 2016

Link to the discussion. I wouldn't object if CTC want's to push this through, just my opinion.

@silverwind

This comment has been minimized.

Contributor

silverwind commented Feb 9, 2016

Also, I'm +1 on documenting it. I'd be happy if you remove the deprecation badge and add a note that it's discouraged.

@vkurchatkin

This comment has been minimized.

Member

vkurchatkin commented Feb 9, 2016

@benjamingr well, I disagree nonetheless)

@thefourtheye

This comment has been minimized.

Contributor

thefourtheye commented Feb 9, 2016

-1. We shouldn't add something that was never public just to say that it's deprecated

@vkurchatkin If we cannot document it, then we cannot deprecate it as per the current deprecation process right?

@jasnell

This comment has been minimized.

Member

jasnell commented Feb 10, 2016

LGTM. This is in line with the CTC discussion and decision.

@benjamingr

This comment has been minimized.

Member

benjamingr commented Feb 10, 2016

I'd like to give @vkurchatkin and @silverwind a chance to express their views and pursue changing the decision first :)

@benjamingr

This comment has been minimized.

Member

benjamingr commented Mar 16, 2016

Hey, I'm not sure how to move forward with this. @vkurchatkin @silverwind is there another way you'd write this? I don't mind following up with a different PR if you have an idea that will convey that status you can agree with.

@silverwind

This comment has been minimized.

Contributor

silverwind commented Mar 16, 2016

Looking at it again, I don't think it's really that much of a big deal calling it 'deprecation' as I may have made it out to be, so LGTM.

jasnell added a commit that referenced this pull request Mar 16, 2016

doc: document deprecation of util._extend
Document that util._extend was never intended to be used
and should be considered deprecated.

PR-URL: #4903
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
@jasnell

This comment has been minimized.

Member

jasnell commented Mar 16, 2016

Landed in d829028

@jasnell jasnell closed this Mar 16, 2016

@jasnell jasnell referenced this pull request Mar 17, 2016

Closed

Planning for v6 #5766

nschonni added a commit to nschonni/node-sass that referenced this pull request Sep 7, 2016

Replace util._extend with Object.assign
This was never part of the public API, and has officially been
depreciated in Node 6 nodejs/node#4903

nschonni added a commit to nschonni/node-sass that referenced this pull request Sep 7, 2016

Replace util._extend with lodash.assign
This was never part of the public API, and has officially been
depreciated in Node 6 nodejs/node#4903
Object.assign can't be used yet since it requires ES2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment