Skip to content

lib: remove Symbol[Async]Dispose polyfills #55276

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 7, 2024
Merged

lib: remove Symbol[Async]Dispose polyfills #55276

merged 1 commit into from
Oct 7, 2024

Conversation

targos
Copy link
Member

@targos targos commented Oct 5, 2024

No description provided.

@targos targos added dont-land-on-v18.x dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Oct 5, 2024
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net
  • @nodejs/streams
  • @nodejs/test_runner

@nodejs-github-bot nodejs-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 Oct 5, 2024
@targos targos mentioned this pull request Oct 5, 2024
Closed
@codecov Codecov
Copy link

codecov bot commented Oct 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.40%. Comparing base (bbdfeeb) to head (9ccc762).
Report is 8 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #55276      +/-   ##
==========================================
- Coverage   88.41%   88.40%   -0.02%     
==========================================
  Files         652      652              
  Lines      186612   186573      -39     
  Branches    36062    36058       -4     
==========================================
- Hits       165001   164943      -58     
- Misses      14885    14894       +9     
- Partials     6726     6736      +10
Files with missing lines Coverage Δ
lib/_http_server.js 96.74% <100.00%> (+0.08%) ⬆️
lib/child_process.js 97.74% <100.00%> (ø)
lib/dgram.js 97.32% <100.00%> (+<0.01%) ⬆️
lib/events.js 99.83% <100.00%> (+<0.01%) ⬆️
lib/https.js 99.30% <100.00%> (ø)
lib/inspector.js 96.27% <100.00%> (-0.06%) ⬇️
lib/internal/child_process.js 95.07% <100.00%> (+<0.01%) ⬆️
lib/internal/events/abort_listener.js 100.00% <100.00%> (ø)
lib/internal/fs/promises.js 98.24% <100.00%> (ø)
lib/internal/http2/core.js 95.53% <100.00%> (ø)
... and 15 more

... and 36 files with indirect coverage changes

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 5, 2024
@nodejs-github-bot
Copy link
Collaborator

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

mcollina
mcollina approved these changes Oct 5, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Lgtm

@avivkeller avivkeller added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 5, 2024
@nodejs-github-bot
Copy link
Collaborator

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

mertcanaltin
mertcanaltin approved these changes Oct 5, 2024
View reviewed changes
Copy link
Member

@mertcanaltin mertcanaltin left a comment

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot
Copy link
Collaborator

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

@atlowChemi atlowChemi added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 7, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 7, 2024
@nodejs-github-bot nodejs-github-bot merged commit d5eb9a3 into nodejs:main Oct 7, 2024
85 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in d5eb9a3

This was referenced Oct 8, 2024
Merged
Merged
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@targos @louwers
lib: remove Symbol[Async]Dispose polyfills
25c7112 
PR-URL: nodejs#55276
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
@targos @tpoisseau
lib: remove Symbol[Async]Dispose polyfills
dd102d8 
PR-URL: nodejs#55276
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@Qard Qard Qard approved these changes

@lpinca lpinca lpinca approved these changes

@anonrig anonrig anonrig approved these changes

@MoLow MoLow MoLow approved these changes

@mertcanaltin mertcanaltin mertcanaltin approved these changes

@atlowChemi atlowChemi atlowChemi 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. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@targos @nodejs-github-bot @mcollina @Qard @lpinca @anonrig @MoLow @mertcanaltin @atlowChemi @avivkeller