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

(v8.x backport) 12712 - util: add util.callbackify() #13750

Closed
wants to merge 4 commits into from
Closed

(v8.x backport) 12712 - util: add util.callbackify() #13750

wants to merge 4 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Jun 17, 2017 •
edited
Loading

Refs: #12712
Refs: #13604
Refs: #13705
(#13705 hasn't landed on master but it's ready)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

util

refack and others added 3 commits June 17, 2017 16:52
@refack
util: add callbackify
13c4fd6 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

PR-URL: nodejs#12712
Fixes: nodejs/CTC#109
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell @refack
errors: add missing ERR_ prefix on util.callbackify error
3643483 
The `FALSY_VALUE_REJECTION` error code added by
nodejs#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

PR-URL: nodejs#13604
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@cjihrig @refack
test: increase util.callbackify() coverage
ace4ade 
This commit adds coverage for util.callbackify() type checking.

PR-URL: nodejs#13705
@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. util Issues and PRs related to the built-in util module. v8.x labels Jun 17, 2017
@refack
Copy link
Contributor Author

refack commented Jun 17, 2017

Gate: https://ci.nodejs.org/job/node-test-commit-linuxone/6709/

@mscdex mscdex removed the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Jun 17, 2017
@refack
Copy link
Contributor Author

refack commented Jun 17, 2017 •
edited
Loading

Gate2: https://ci.nodejs.org/job/node-test-commit-linuxone/6710/ ✔️

@refack
Copy link
Contributor Author

refack commented Jun 17, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/8717/

@refack refack mentioned this pull request Jun 17, 2017
Merged
3 tasks
@addaleax
Copy link
Member

(#13705 hasn't landed on master but it's ready)

I’ll still wait for it to land then

@refack
Copy link
Contributor Author

refack commented Jun 20, 2017

(#13705 hasn't landed on master but it's ready)

I’ll still wait for it to land then

Landed, so this is ready.

addaleax
addaleax approved these changes Jun 20, 2017
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM, but there was no need for a backport of #13705 so I’ll not land that and instead cherry-pick it as usual

addaleax pushed a commit that referenced this pull request Jun 20, 2017
@refack @addaleax
util: add callbackify
eec6317 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jun 20, 2017
@jasnell @addaleax
errors: add missing ERR_ prefix on util.callbackify error
eb6817d 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax
Copy link
Member

Landed in eec6317, eb6817d

@addaleax addaleax closed this Jun 20, 2017
@refack
Copy link
Contributor Author

refack commented Jun 20, 2017 •
edited
Loading

and instead cherry-pick it as usual

cool.

@addaleax addaleax mentioned this pull request Jun 21, 2017
Merged
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@refack @addaleax
util: add callbackify
66349af 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jun 21, 2017
@jasnell @addaleax
errors: add missing ERR_ prefix on util.callbackify error
ed01bfe 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jun 24, 2017
@refack @addaleax
util: add callbackify
b879ab7 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jun 24, 2017
@jasnell @addaleax
errors: add missing ERR_ prefix on util.callbackify error
efaa03c 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this pull request Jun 29, 2017
@refack @rvagg
util: add callbackify
e2da7c4 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this pull request Jun 29, 2017
@jasnell @rvagg
errors: add missing ERR_ prefix on util.callbackify error
508a822 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@refack @addaleax
util: add callbackify
041b2b2 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jul 11, 2017
@jasnell @addaleax
errors: add missing ERR_ prefix on util.callbackify error
0264a3a 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@refack @addaleax
util: add callbackify
8cba959 
Add `util.callbackify(function)` for creating callback style functions
from functions returning a `Thenable`

Original-PR-URL: #12712
Fixes: nodejs/CTC#109
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Original-Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Original-Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax pushed a commit that referenced this pull request Jul 18, 2017
@jasnell @addaleax
errors: add missing ERR_ prefix on util.callbackify error
e8780ba 
The `FALSY_VALUE_REJECTION` error code added by
#12712 did not have the `ERR_` prefix,
nor was it added to the errors.md documentation. Add the prefix in for
consistency.

Original-PR-URL: #13604
Original-Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Original-Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Original-Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original-Reviewed-By: Refael Ackermann <refack@gmail.com>
Original-Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Original-Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

PR-URL: #13750
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR

@MylesBorins MylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Aug 14, 2017
@refack
Copy link
Contributor Author

refack commented Aug 14, 2017

My intuition is if we don't backport promisify than no need to backport this...
/cc @Fishrock123 RE: junosuarez/callbackify#5

@refack refack deleted the backport-12712-to-v8.x branch August 15, 2017 18:08
@refack
Copy link
Contributor Author

refack commented Sep 3, 2017

@benjamingr?

@benjamingr
Copy link
Member

I'm not sure, this is an API change that is mostly reactive to a feature 6.x doesn't have (async/await).

I'm fine with backporting it it's just not really required IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

Assignees
No one assigned
Labels
semver-minor PRs that contain new features and should be released in the next minor version. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@refack @addaleax @MylesBorins @benjamingr @mscdex @nodejs-github-bot @jasnell @cjihrig