-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: fix links in YAML metadata of assert.md #18670
Conversation
doc/api/assert.md
Outdated
@@ -208,7 +208,7 @@ changes: | |||
description: Enumerable symbol properties are now compared. | |||
- version: v9.0.0 | |||
pr-url: https://github.com/nodejs/node/pull/15036 | |||
description: NaN is now compared using the [SameValueZero][] comparison. | |||
description: NaN is now compared using the [SameValue Comparison][]. |
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.
That is actually not correct. At that point of time it was a SameValueZero
comparison. The same applies to the other entry.
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.
Sorry. What link should I add to the bottom list? There is no [SameValueZero]
there.
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.
The linter does not recognize the references in the comment. Therefore it would complain in case you want to add it. At least that was the case why the link was originally removed.
The best solution out of my perspective is to just remove the reference and keep the plain SameValueZero
and maybe add backticks around it.
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.
Can we inline the link like [SameValueZero](https://tc39.github.io/ecma262/#sec-samevaluezero)
?
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.
That should be possible as far as I can tell and that is probably even a better solution.
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.
I've amended, but we have line length more than 80 characters now. Let us see if linter is OK with that...
PR-URL: #18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Landed in 8e70811 |
PR-URL: #18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#18670 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
doc, assert