Skip to content

lib, tools: remove duplicate requires#54987

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
Merged
nodejs-github-bot merged 1 commit into from
Sep 25, 2024
Merged

lib, tools: remove duplicate requires #54987

nodejs-github-bot merged 1 commit into from
Sep 25, 2024

Conversation

@avivkeller
Copy link
Member

@avivkelleravivkeller commented Sep 17, 2024•
edited
Loading

Updates the eslint rule to check if a require in a function is already defined at the top-level.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 17, 2024

Review requested:

  • @nodejs/loaders
  • @nodejs/startup

@nodejs-github-botnodejs-github-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 Sep 17, 2024
@avivkelleravivkeller added the tools Issues and PRs related to the tools directory. label Sep 17, 2024
avivkeller
avivkeller commented Sep 17, 2024
View reviewed changes
@codecov
Copy link

codecovbot commented Sep 17, 2024•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.24%. Comparing base (76edde5) to head (a572215).
Report is 404 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #54987 +/- ## ========================================== - Coverage 88.26% 88.24% -0.02%  ========================================== Files 651 651 Lines 183894 183873 -21 Branches 35858 35851 -7 ========================================== - Hits 162315 162266 -49 - Misses 14882 14894 +12 - Partials 6697 6713 +16
Files with missing linesCoverage Δ
lib/events.js99.83% <100.00%> (-0.01%)⬇️
lib/internal/abort_controller.js98.00% <100.00%> (-0.03%)⬇️
...ernal/bootstrap/switches/does_own_process_state.js96.52% <100.00%> (+0.63%)⬆️
lib/internal/main/worker_thread.js95.47% <100.00%> (ø)
lib/internal/modules/cjs/loader.js97.36% <100.00%> (-0.01%)⬇️
lib/internal/modules/esm/loader.js98.34% <100.00%> (+<0.01%)⬆️
lib/internal/modules/esm/resolve.js96.62% <ø> (-0.01%)⬇️
lib/internal/process/pre_execution.js93.38% <100.00%> (-0.44%)⬇️
lib/internal/process/warning.js99.04% <100.00%> (+1.35%)⬆️

... and 27 files with indirect coverage changes

@avivkelleravivkeller mentioned this pull request Sep 21, 2024
Merged
@avivkelleravivkeller added the blocked PRs that are blocked by other issues or PRs. label Sep 21, 2024
@avivkelleravivkellerforce-pushed the remove-duplicate-requires branch from d7aee31 to a572215CompareSeptember 23, 2024 20:30
@avivkelleravivkeller added request-ci Add this label to start a Jenkins CI on a PR. and removed blocked PRs that are blocked by other issues or PRs. labels Sep 23, 2024
@avivkeller
Copy link
MemberAuthor

avivkeller commented Sep 23, 2024

No longer blocked.

@avivkelleravivkeller added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 23, 2024
@github-actionsgithub-actionsbot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Sep 23, 2024
@github-actions
Copy link
Contributor

github-actionsbot commented Sep 23, 2024

Failed to start CI
 ⚠ Something was pushed to the Pull Request branch since the last approving review. ✘ Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/11001927470

@aduh95aduh95 added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Sep 23, 2024
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 23, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 23, 2024

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

@avivkeller
Copy link
MemberAuthor

avivkeller commented Sep 24, 2024

CI is 🟢 / 🟠 (2 + 1 Flakes)

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 24, 2024

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

@avivkeller
Copy link
MemberAuthor

avivkeller commented Sep 25, 2024

CI is 🟢 🎉

@aduh95aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 25, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 25, 2024
@nodejs-github-botnodejs-github-bot merged commit 5c22d19 into nodejs:mainSep 25, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 25, 2024

Landed in 5c22d19

targos pushed a commit that referenced this pull request Oct 4, 2024
@avivkeller@targos
lib, tools: remove duplicate requires
566179c
PR-URL: #54987 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@aduh95aduh95 mentioned this pull request Oct 9, 2024
Merged
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@avivkeller@louwers
lib, tools: remove duplicate requires
bb7f16d
PR-URL: nodejs#54987 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@marco-ippolitomarco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
@avivkeller@tpoisseau
lib, tools: remove duplicate requires
88f6f7f
PR-URL: nodejs#54987 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.dont-land-on-v20.xPRs that should not land on the v20.x-staging branch and should not be released in v20.x.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.toolsIssues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@avivkeller@nodejs-github-bot@jasnell@aduh95@marco-ippolito