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

src: make some minor ToLocalChecked cleanups #56483

Merged
merged 1 commit into from
Jan 7, 2025
Merged

src: make some minor ToLocalChecked cleanups #56483

merged 1 commit into from
Jan 7, 2025
+27 −21

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 5, 2025

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run. labels Jan 5, 2025
@jasnell jasnell requested review from anonrig and aduh95 January 5, 2025 21:14
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@codecov Codecov
Copy link

codecov bot commented Jan 5, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 54.54545% with 10 lines in your changes missing coverage. Please review.

Project coverage is 88.52%. Comparing base (b736028) to head (f7a524b).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/cares_wrap.cc 54.54% 1 Missing and 9 partials ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #56483      +/-   ##
==========================================
- Coverage   88.52%   88.52%   -0.01%     
==========================================
  Files         660      660              
  Lines      190885   190892       +7     
  Branches    36629    36633       +4     
==========================================
+ Hits       168982   168984       +2     
+ Misses      15094    15093       -1     
- Partials     6809     6815       +6
Files with missing lines Coverage Δ
src/cares_wrap.cc 65.04% <54.54%> (-0.21%) ⬇️

... and 22 files with indirect coverage changes

@daeyeon daeyeon added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 6, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
addaleax
addaleax approved these changes Jan 7, 2025
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

💙

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 7, 2025 •
edited by jasnell
Loading

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

@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Jan 7, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 7, 2025
@nodejs-github-bot nodejs-github-bot merged commit 3b5f235 into nodejs:main Jan 7, 2025
74 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 3b5f235

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daeyeon daeyeon daeyeon approved these changes

@anonrig anonrig anonrig approved these changes

@addaleax addaleax addaleax approved these changes

@aduh95 aduh95 Awaiting requested review from aduh95

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jasnell @nodejs-github-bot @addaleax @anonrig @daeyeon