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

doc: make clear the result of comparison between Symbol.for #43309

Merged
merged 1 commit into from
Jun 6, 2022
Merged

doc: make clear the result of comparison between Symbol.for #43309

merged 1 commit into from
Jun 6, 2022
+1 −1

Conversation

cola119
Copy link
Member

@cola119 cola119 commented Jun 4, 2022 •
edited
Loading

This PR made clear the execution result of the inline script in the same way as L22 to be able to understand the difference of Symbol() and Symbol.for() at a glance.

node/doc/contributing/using-symbols.md

Lines 20 to 22 in dfa896f

const kField = Symbol('kField');
console.log(kField === Symbol('kField')); // false

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 4, 2022
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 4, 2022
mcollina
mcollina approved these changes Jun 5, 2022
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@marsonya marsonya added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 5, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 5, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43309
✔  Done loading data for nodejs/node/pull/43309
----------------------------------- PR info ------------------------------------
Title      doc: make clear the result of comparison between Symbol.for (#43309)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     cola119:make-symbol-doc-clear -> nodejs:master
Labels     doc, author ready
Commits    1
 - doc: make clear the result of comparison between Symbol.for
Committers 1
 - cola119 
PR-URL: https://github.com/nodejs/node/pull/43309
Reviewed-By: Tobias Nießen 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Colin Ihrig 
Reviewed-By: Matteo Collina 
Reviewed-By: Akhil Marsonya 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43309
Reviewed-By: Tobias Nießen 
Reviewed-By: Antoine du Hamel 
Reviewed-By: Colin Ihrig 
Reviewed-By: Matteo Collina 
Reviewed-By: Akhil Marsonya 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sat, 04 Jun 2022 00:48:07 GMT
   ✔  Approvals: 5
   ✔  - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/43309#pullrequestreview-995645903
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/43309#pullrequestreview-995792677
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/43309#pullrequestreview-995943669
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/43309#pullrequestreview-995984749
   ✔  - Akhil Marsonya (@marsonya): https://github.com/nodejs/node/pull/43309#pullrequestreview-996026290
   ✖  This PR needs to wait 6 more hours to land
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2443883619

@nodejs-github-bot nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jun 5, 2022
@cola119
Copy link
Member Author

cola119 commented Jun 6, 2022

CI failed by ✖ This PR needs to wait 6 more hours to land

@RaisinTen RaisinTen added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 6, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 6, 2022
@nodejs-github-bot nodejs-github-bot merged commit 18a0ead into nodejs:master Jun 6, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 18a0ead

@cola119 cola119 deleted the make-symbol-doc-clear branch June 6, 2022 13:29
danielleadams pushed a commit that referenced this pull request Jun 11, 2022
@cola119 @danielleadams
doc: make clear the result of comparison between Symbol.for
3581400 
PR-URL: #43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 11, 2022
Merged
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
@cola119 @danielleadams
doc: make clear the result of comparison between Symbol.for
8e03c7f 
PR-URL: #43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams pushed a commit that referenced this pull request Jun 13, 2022
@cola119 @danielleadams
doc: make clear the result of comparison between Symbol.for
05f38c6 
PR-URL: #43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos pushed a commit that referenced this pull request Jul 12, 2022
@cola119 @targos
doc: make clear the result of comparison between Symbol.for
7f38117 
PR-URL: #43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2022
@cola119 @targos
doc: make clear the result of comparison between Symbol.for
3d47c66 
PR-URL: #43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@cola119 @guangwong
doc: make clear the result of comparison between Symbol.for
7f7c061 
PR-URL: nodejs/node#43309
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@cjihrig cjihrig cjihrig approved these changes

@tniessen tniessen tniessen approved these changes

@aduh95 aduh95 aduh95 approved these changes

@marsonya marsonya marsonya approved these changes

@RaisinTen RaisinTen RaisinTen approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@cola119 @nodejs-github-bot @mcollina @cjihrig @tniessen @aduh95 @marsonya @RaisinTen