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

tools: improve valid-typeof lint rule #37924

Merged
merged 0 commits into from
Mar 28, 2021
Merged

tools: improve valid-typeof lint rule #37924

merged 0 commits into from
Mar 28, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 26, 2021

Require that typeof comparisons be to string literals.

Fix an instance in a test where it was mistakenly being compared to undefined rather than 'undefined'.

@nodejs-github-bot nodejs-github-bot added encoding Issues and PRs related to the TextEncoder and TextDecoder APIs. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Mar 26, 2021
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/36987/

aduh95
aduh95 approved these changes Mar 26, 2021
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

+1, although it seems to have broken the assert.CallTracker test.

@Trott
Copy link
Member Author

Trott commented Mar 26, 2021

+1, although it seems to have broken the assert.CallTracker test.

Ha! The test was written wrong, but it "worked" because it made two errors that canceled each other out. It should be exiting if it's not undefined. Will rewrite using assert.strictEqual() so things are more clear.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/36995/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/36999/

Trott added a commit to Trott/io.js that referenced this pull request Mar 28, 2021
@Trott
test: fix typeof comparison
f34e88a 
The typeof operator returns a string. Compare to the string 'undefined'
and not the value undefined.

PR-URL: nodejs#37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@Trott Trott closed this Mar 28, 2021
@Trott Trott merged commit adcd60e into nodejs:master Mar 28, 2021
@Trott
Copy link
Member Author

Trott commented Mar 28, 2021

Landed in db7df59...adcd60e

@Trott Trott deleted the valid-typeof branch March 28, 2021 15:02
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
@Trott @ruyadorno
test: fix typeof comparison
5b01056 
The typeof operator returns a string. Compare to the string 'undefined'
and not the value undefined.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Mar 29, 2021
@Trott @ruyadorno
tools: improve valid-typeof lint rule
2a7e0a3 
Require that `typeof` comparisons be to string literals.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
@Trott @ruyadorno
test: fix typeof comparison
e256c4d 
The typeof operator returns a string. Compare to the string 'undefined'
and not the value undefined.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this pull request Mar 30, 2021
@Trott @ruyadorno
tools: improve valid-typeof lint rule
b7e7384 
Require that `typeof` comparisons be to string literals.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@ruyadorno ruyadorno mentioned this pull request Mar 30, 2021
Merged
targos pushed a commit that referenced this pull request May 1, 2021
@Trott @targos
test: fix typeof comparison
7b0e4e2 
The typeof operator returns a string. Compare to the string 'undefined'
and not the value undefined.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request May 1, 2021
@Trott @targos
tools: improve valid-typeof lint rule
4254315 
Require that `typeof` comparisons be to string literals.

PR-URL: #37924
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
encoding Issues and PRs related to the TextEncoder and TextDecoder APIs. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Trott @nodejs-github-bot @jasnell @targos @cjihrig @BridgeAR @aduh95