-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
Obvious, adding return type to util.promisify(), but valid update to … #16040
Conversation
…documentation for my first commit
@@ -461,6 +461,7 @@ added: v8.0.0 | |||
--> | |||
|
|||
* `original` {Function} | |||
Returns: {Function} a promise style function |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is somewhat inconsistent throughout this documentation how we do this but most cases use the star to better outline the return value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the end of the Style Guide: https://github.com/nodejs/node/blob/master/doc/STYLE_GUIDE.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"promise style function" doesn't really work in this regard, either this should be just {Function}
as the return type or the "original" line should be expanded as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with @gibfahn's and @benjamingr's comments addressed.
That is: * Returns: {Function}
Landed in 9c44215 Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 ! |
PR-URL: #16040 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #16040 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs/node#16040 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs/node#16040 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
adding return type to util.promisify()
Checklist
Affected core subsystem(s)
doc