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

[v8.x backport] doc: removed redundant mentions to error codes #14175

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jul 11, 2017

Backport of #13627

PR-URL: #13627
Reviewed-By: Daniel Bevenius daniel.bevenius@gmail.com
Reviewed-By: Rich Trott rtrott@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Timothy Gu timothygu99@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

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

doc

PR-URL: nodejs#13627
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. v8.x labels Jul 11, 2017
@Trott
Copy link
Member Author

Trott commented Jul 11, 2017

@addaleax

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

Unless I'm wrong


<a id="ERR_FALSY_VALUE_REJECTION"></a>
### ERR_FALSY_VALUE_REJECTION

Used by the `util.callbackify()` API when a callbackified `Promise` is rejected
with a falsy value (e.g. `null`).
The `ERR_FALSY_VALUE_REJECTION` error code is used by the `util.callbackify()`
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this one wrong way around?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, was too hasty. Thanks for the catch.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed and pushed.

value]` tuple – that is, if an element is not iterable, or does not consist of
exactly two elements.
Used when an element in the `iterable` provided to the [WHATWG][WHATWG URL
API] [`URLSearchParams`constructor][`new URLSearchParams(iterable)`] does not
Copy link
Member

Choose a reason for hiding this comment

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

There should be a space between "URLSearchParams" and "constructor".

Copy link
Contributor

Choose a reason for hiding this comment

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

Should, but by the magic of GFM it works:
image

Copy link
Member

Choose a reason for hiding this comment

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

@refack what you are seeing is just GitHub's CSS. This won't be the case on Node.js' website. Even in GH, copying the text via the clipboard will not contain a space, nor will the browser insert a line break even if one is needed there.

Copy link
Member Author

Choose a reason for hiding this comment

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

This issue is in master. I guess it should be addressed with a PR there that then gets backported to 8.x.

Copy link
Member

Choose a reason for hiding this comment

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

@Trott Alright, I've opened #14181.

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

I'd prefer to just squash in #14181 with this PR, but LGTM anyway.

addaleax pushed a commit that referenced this pull request Jul 12, 2017
PR-URL: #13627
Backport-PR-URL: #14175
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@addaleax
Copy link
Member

Thanks for the backport! Landed in bfb2db6 @ v8.x-staging

I'd prefer to just squash in #14181 with this PR, but LGTM anyway.

There’s not really any difference. ¯\_(ツ)_/¯

@addaleax addaleax closed this Jul 12, 2017
addaleax pushed a commit that referenced this pull request Jul 18, 2017
PR-URL: #13627
Backport-PR-URL: #14175
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017
PR-URL: #13627
Backport-PR-URL: #14175
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@Trott Trott deleted the backport-13627 branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants