doc: add note regarding %Array.prototype.concat% in primordials.md#43166
Merged
nodejs-github-bot merged 4 commits intonodejs:masterfrom May 29, 2022
Merged
Conversation
Collaborator
|
Review requested:
|
Co-authored-by: Livia Medeiros <74449973+LiviaMedeiros@users.noreply.github.com>
LiviaMedeiros
approved these changes
May 22, 2022
aduh95
added
the
author ready
RaisinTen
approved these changes
May 22, 2022
Member
RaisinTen
left a comment
RaisinTen
left a comment
There was a problem hiding this comment.
I think it would also be useful to specify that the first example is unsafe and the second one is safe, so that folks reading this doc could use what's used in the second example while replacing uses of ArrayPrototypeConcat / Array.prototype.concat. Other than that, looks good!
Co-authored-by: Darshan Sen <raisinten@gmail.com>
LiviaMedeiros
added
commit-queue
nodejs-github-bot
added
commit-queue-failed
Collaborator
Commit Queue failed- Loading data for nodejs/node/pull/43166 ✔ Done loading data for nodejs/node/pull/43166 ----------------------------------- PR info ------------------------------------ Title doc: add note regarding `%Array.prototype.concat%` in `primordials.md` (#43166) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch aduh95:array-concat-prototype-tampering -> nodejs:master Labels doc, author ready, commit-queue-squash Commits 4 - doc: add note regarding `%Array.prototype.concat%` in `primordials.md` - fixup! doc: add note regarding `%Array.prototype.concat%` in `primord… - Apply suggestions from code review - Apply suggestions from code review Committers 2 - Antoine du Hamel - GitHub PR-URL: https://github.com/nodejs/node/pull/43166 Reviewed-By: LiviaMedeiros Reviewed-By: Darshan Sen ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/43166 Reviewed-By: LiviaMedeiros Reviewed-By: Darshan Sen -------------------------------------------------------------------------------- ⚠ Commits were pushed since the last review: ⚠ - Apply suggestions from code review ℹ This PR was created on Sat, 21 May 2022 10:50:31 GMT ✔ Approvals: 2 ✔ - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/43166#pullrequestreview-980911633 ✔ - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/43166#pullrequestreview-980961722 ✔ Last GitHub CI successful ℹ Green GitHub CI is sufficient -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/2403578118 |
LiviaMedeiros
approved these changes
May 29, 2022
LiviaMedeiros
added
commit-queue
mcollina
reviewed
May 29, 2022
Member
mcollina
left a comment
mcollina
left a comment
There was a problem hiding this comment.
Is it already noted that this will be significantly slower?
LiviaMedeiros
removed
the
commit-queue
Collaborator
|
Landed in df56644 |
Member
|
@mcollina Ugh, probably not. Was there a prior discussion (or measurements) about that? |
bengl
pushed a commit
that referenced
this pull request
May 30, 2022
PR-URL: #43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Merged
Contributor
Author
I mean, node/doc/contributing/primordials.md Line 118 in 52ba02e IIRC, |
F3n67u
pushed a commit
to F3n67u/node
that referenced
this pull request
Jun 1, 2022
PR-URL: nodejs#43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
danielleadams
pushed a commit
that referenced
this pull request
Jun 27, 2022
PR-URL: #43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos
pushed a commit
that referenced
this pull request
Jul 12, 2022
PR-URL: #43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
targos
pushed a commit
that referenced
this pull request
Jul 31, 2022
PR-URL: #43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
guangwong
pushed a commit
to noslate-project/node
that referenced
this pull request
Oct 10, 2022
PR-URL: nodejs/node#43166 Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Darshan Sen <raisinten@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.