Skip to content

doc: change Node.js style to error-first style - #17638

Closed
lossycache wants to merge 4 commits into
nodejs:masterfrom
lossycache:doc-error-first-callback
Closed

doc: change Node.js style to error-first style#17638
lossycache wants to merge 4 commits into
nodejs:masterfrom
lossycache:doc-error-first-callback

Conversation

@lossycache

Copy link
Copy Markdown
Contributor

We change the awkward "Node.js style callback" phrasing to the
more informative "error-first style callback," which is more
in line with its usage

Refs: #17593 (comment)

Checklist
Affected core subsystem(s)

doc

@nodejs-github-botnodejs-github-bot added the doc Issues and PRs related to the documentations. label Dec 12, 2017
@lossycache

Copy link
Copy Markdown
ContributorAuthor

I think these were the only two files (errors.md and util.md) where Node.js style callback was used, please comment if there are any other places where this change can be made 😄

@vsemozhetbytvsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Dec 13, 2017

@TrottTrott left a comment

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.

Looks good to me! Would like it even more if we go further and remove style so it's just error-first callback and not error-first style callback.

Comment threaddoc/api/errors.md Outdated

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.

Super-tiny nit here, and I know this was in the original text and not something you introduced, so feel free to ignore this, but it would be good to replace the quotation marks in this line with italics because that's the standard typography for words-as-words. (See https://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style/Text_formatting#Words_as_words for more information if curious.)

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

👍 no problem

@maclover7

Copy link
Copy Markdown
Contributor

FYI -- triggered CI but Jenkins is being a little weird right now. Don't have a link to paste yet.

@vsemozhetbyt

vsemozhetbyt commented Dec 13, 2017

Copy link
Copy Markdown
Contributor

@BridgeARBridgeAR left a comment

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.

LGTM even though I would like my comment to be incorporated.

Comment threaddoc/api/errors.md Outdated

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.

For historical reasons it would be good to keep a reference to the Node.js style callback as in:

... error-first callback, historically called `Node.js style callback`.

The reason is that e.g. blog posts and other documentations might still call them like this and it would be good to find a explanation even with the new name.

@TrottTrottDec 13, 2017

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.

Rather than historically I'd prefer the simpler and arguably-more-accurate sometimes...

... an _error-first callback_ (sometimes referred to as a _Node.js style callback_) ...

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

sounds correct to me as well. Done 👍

We change the awkward "Node.js style callback" phrasing to the
more informative "error-first style callback," which is more
in line with its usage
Refs: nodejs#17593 (comment)
@lossycache
lossycacheforce-pushed the doc-error-first-callback branch from 5db6ae5 to 9f13040CompareDecember 13, 2017 18:57
@apapirovski

Copy link
Copy Markdown
Contributor

apapirovski pushed a commit that referenced this pull request Dec 17, 2017
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: #17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@apapirovski

Copy link
Copy Markdown
Contributor

Landed in 3db136a. Thanks @ramsgoli!

MylesBorins pushed a commit that referenced this pull request Jan 8, 2018
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: #17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 9, 2018
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: #17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jan 10, 2018
boingoing pushed a commit to nodejs/node-chakracore that referenced this pull request Jan 18, 2018
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: nodejs/node#17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 23, 2018
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: #17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
gibfahn pushed a commit that referenced this pull request Jan 24, 2018
Change the awkward "Node.js style callback" phrasing to the more
informative "error-first callback."
PR-URL: #17638
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.fast-trackPRs that do not need to wait for 48 hours to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@lossycache@maclover7@vsemozhetbyt@apapirovski@jasnell@Trott@Fishrock123@lpinca@cjihrig@gireeshpunathil@BridgeAR@MylesBorins@nodejs-github-bot