Skip to content

Conversation

@legendecas
Copy link
Member

Fixes: #60589

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net
  • @nodejs/realm
  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. 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 Nov 6, 2025
@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 91.17647% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.54%. Comparing base (b4b1413) to head (5ff1880).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/js_udp_wrap.cc 0.00% 1 Missing ⚠️
src/node_snapshotable.cc 80.00% 0 Missing and 1 partial ⚠️
src/stream_base-inl.h 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #60602      +/-   ##
==========================================
- Coverage   88.56%   88.54%   -0.03%     
==========================================
  Files         704      704              
  Lines      208077   208085       +8     
  Branches    40084    40082       -2     
==========================================
- Hits       184289   184253      -36     
- Misses      15826    15867      +41     
- Partials     7962     7965       +3     
Files with missing lines Coverage Δ
src/base_object-inl.h 82.70% <100.00%> (+0.75%) ⬆️
src/base_object.cc 84.31% <100.00%> (-0.99%) ⬇️
src/base_object.h 100.00% <ø> (ø)
src/cppgc_helpers-inl.h 84.00% <100.00%> (ø)
src/env-inl.h 94.45% <100.00%> (-0.02%) ⬇️
src/env.cc 85.07% <100.00%> (-0.38%) ⬇️
src/histogram.cc 78.96% <100.00%> (-0.06%) ⬇️
src/node_context_data.h 83.33% <100.00%> (ø)
src/node_contextify.cc 81.78% <ø> (-0.09%) ⬇️
src/node_process_methods.cc 88.69% <100.00%> (ø)
... and 5 more

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

RSLGTM. I feel that it'd be nicer to add a method to BaseObject that allows setting arbitrary slot with the tag so that we don't have to repeat the default argument in all its subclasses, but that can be a followup

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2025
@legendecas
Copy link
Member Author

I feel that it'd be nicer to add a method to BaseObject that allows setting arbitrary slot with the tag so that we don't have to repeat the default argument in all its subclasses, but that can be a followup.

Many of the changed lines in this PR are not in the subclasses of BaseObject. We might need a standalone helper util if it makes sense.

@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. 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

Development

Successfully merging this pull request may close these issues.

V8's aligned pointer APIs are deprecated

7 participants