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: add note to util.isBuffer #3790

Merged
merged 1 commit into from Nov 12, 2015

Conversation

Projects
None yet
8 participants
@evanlucas
Copy link
Member

commented Nov 12, 2015

Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

@mscdex mscdex added util doc labels Nov 12, 2015

@Fishrock123

This comment has been minimized.

Copy link
Member

commented Nov 12, 2015

LGTM

@Fishrock123

This comment has been minimized.

Copy link
Member

commented Nov 12, 2015

Of note though, it's just an instanceof check that would probably be more efficient inlined.

@pmq20

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

Since it's deprecated why still keep the usage description of the old API? Shall we reduce to only the note of "Use Buffer.isBuffer() instead."?

@cjihrig

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

LGTM. The message might be better with the description of the function, above the code example.

@thefourtheye

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

If it is deprecated can we use the stability note like in other functions in it?

@jasnell

This comment has been minimized.

Copy link
Member

commented Nov 12, 2015

LGTM

@evanlucas

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2015

@cjihrig are you saying move the note to the top of the section?

@evanlucas

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2015

@pmq20 well, it hasn't been removed yet. I think we should keep that part until we actually remove the api

@cjihrig

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

@evanlucas I meant move it from line 418 to line 407. If you think it would be more likely to be noticed.

@evanlucas

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2015

yea, looking back at it, I think that makes more sense

@evanlucas evanlucas force-pushed the evanlucas:isbufferdocs branch from 6ddf6bb Nov 12, 2015

@evanlucas

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2015

Updated. PTAL

@cjihrig

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

LGTM

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

@evanlucas evanlucas force-pushed the evanlucas:isbufferdocs branch to 7cb3a54 Nov 12, 2015

@evanlucas evanlucas closed this Nov 12, 2015

@evanlucas evanlucas deleted the evanlucas:isbufferdocs branch Nov 12, 2015

@evanlucas evanlucas merged commit 7cb3a54 into nodejs:master Nov 12, 2015

@evanlucas

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2015

Landed in 7cb3a54. Thanks!

@thefourtheye

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

Ah... The deprecation notice is already there. Sorry about the fuss

evanlucas added a commit that referenced this pull request Nov 13, 2015

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

evanlucas added a commit that referenced this pull request Nov 17, 2015

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins

This comment has been minimized.

Copy link
Member

commented Nov 17, 2015

landed in v4.x-staging in 3435f87

evanlucas added a commit that referenced this pull request Dec 4, 2015

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

@jasnell jasnell referenced this pull request Dec 17, 2015

Closed

Release 4.2.4 Planning #4321

evanlucas added a commit that referenced this pull request Dec 17, 2015

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

evanlucas added a commit that referenced this pull request Dec 23, 2015

doc: add note to util.isBuffer
Since util.isBuffer is deprecated, we should be explicit that
Buffer.isBuffer should be used instead.

PR-URL: #3790
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.