Skip to content

Conversation

@Trott
Copy link
Member

@TrottTrott commented Jan 2, 2021

Now that we are using primordials in the first part of
isIdenticalTypedArrayType(), the defensive coding to get the correct
result (when Symbol.toStringTag is manipulated) is no longer reachable
or necessary. Remove the code.

Refs: https://coverage.nodejs.org/coverage-873d21cdc1266273/lib/internal/util/comparisons.js.html#L135

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the util Issues and PRs related to the built-in util module. label Jan 2, 2021
@TrottTrott requested a review from BridgeARJanuary 2, 2021 19:05
@TrottTrott added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 3, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Now that we are using primordials in the first part of isIdenticalTypedArrayType(), the defensive coding to get the correct result (when Symbol.toStringTag is manipulated) is no longer reachable or necessary. Remove the code. Refs: https://coverage.nodejs.org/coverage-873d21cdc1266273/lib/internal/util/comparisons.js.html#L135 PR-URL: nodejs#36744 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@TrottTrott merged commit ca86e34 into nodejs:masterJan 5, 2021
@Trott
Copy link
MemberAuthor

Trott commented Jan 5, 2021

Landed in ca86e34

@TrottTrott deleted the not-dead branch January 5, 2021 05:00
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Now that we are using primordials in the first part of isIdenticalTypedArrayType(), the defensive coding to get the correct result (when Symbol.toStringTag is manipulated) is no longer reachable or necessary. Remove the code. Refs: https://coverage.nodejs.org/coverage-873d21cdc1266273/lib/internal/util/comparisons.js.html#L135 PR-URL: #36744 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Jan 12, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@Trott@nodejs-github-bot@jasnell@cjihrig@BridgeAR@Lxxyx@aduh95@targos