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

lib: harden lint checks for globals #38419

Closed
wants to merge 2 commits into from
Closed

lib: harden lint checks for globals #38419

wants to merge 2 commits into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 26, 2021

In most cases we want to avoid relying of looking up the global scope to access an API, which could be mutated from userland.

@github-actions github-actions bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 26, 2021
@aduh95 aduh95 added the review wanted PRs that need reviews. label Apr 28, 2021
@aduh95
Copy link
Contributor Author

aduh95 commented Apr 28, 2021

@nodejs/linting wdyt?

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 28, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 28, 2021 •
edited by jasnell
Loading

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

@jasnell jasnell removed the needs-ci PRs that need a full CI run. label Apr 28, 2021
@jasnell
Copy link
Member

jasnell commented Apr 28, 2021

Landed in 7919ced

@jasnell jasnell closed this Apr 28, 2021
jasnell pushed a commit that referenced this pull request Apr 28, 2021
@aduh95 @jasnell
lib: harden lint checks for globals
7919ced 
PR-URL: #38419
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
targos pushed a commit that referenced this pull request Apr 29, 2021
@aduh95 @targos
lib: harden lint checks for globals
fcac2e0 
PR-URL: #38419
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@targos targos mentioned this pull request May 3, 2021
Merged
@aduh95 aduh95 deleted the linter-no-globals branch July 19, 2021 14:59
aduh95 added a commit to aduh95/node that referenced this pull request Jul 19, 2021
@aduh95
lib: harden lint checks for globals
cacbccb 
PR-URL: nodejs#38419
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@aduh95 aduh95 mentioned this pull request Jul 19, 2021
Closed
richardlau pushed a commit that referenced this pull request Jul 20, 2021
@aduh95 @richardlau
lib: harden lint checks for globals
0d2b4b7 
PR-URL: #38419
Backport-PR-URL: #39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@richardlau
Copy link
Member

Backported to v14.x-staging in #39448.

richardlau pushed a commit that referenced this pull request Jul 20, 2021
@aduh95 @richardlau
lib: harden lint checks for globals
e3223ed 
PR-URL: #38419
Backport-PR-URL: #39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
@richardlau richardlau mentioned this pull request Jul 20, 2021
Merged
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@aduh95 @foxxyz
lib: harden lint checks for globals
3e7ed33 
PR-URL: nodejs#38419
Backport-PR-URL: nodejs#39448
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
aduh95 added a commit to aduh95/node that referenced this pull request Feb 16, 2022
@aduh95
tools,lib: remove global and Intl from the list of globals
4d3de28 
Refs: nodejs#38230
Refs: nodejs#38419
@aduh95 aduh95 mentioned this pull request Feb 16, 2022
Merged
nodejs-github-bot pushed a commit that referenced this pull request Feb 18, 2022
@aduh95
tools,lib: remove global and Intl from the list of globals
3aab986 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
53bbbc8 
Refs: nodejs#38230
Refs: nodejs#38419

PR-URL: nodejs#42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
749b4fd 
Refs: nodejs#38230
Refs: nodejs#38419

PR-URL: nodejs#42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
1ea4726 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 21, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
a87fa5b 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
bengl pushed a commit that referenced this pull request Feb 22, 2022
@aduh95 @bengl
tools,lib: remove global and Intl from the list of globals
cfafb45 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 21, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
1dfbf3e 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 24, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
80f944f 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 24, 2022
@aduh95 @danielleadams
tools,lib: remove global and Intl from the list of globals
4bba279 
Refs: #38230
Refs: #38419

PR-URL: #42014
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Shingo Inoue <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaisinTen RaisinTen RaisinTen approved these changes

@jasnell jasnell jasnell 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. lib / src Issues and PRs related to general changes in the lib or src directory. review wanted PRs that need reviews.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aduh95 @nodejs-github-bot @jasnell @richardlau @RaisinTen @targos