Skip to content

src: allow std::string_view arguments to SPrintF() and friends - #60058

Closed
addaleax wants to merge 1 commit into
nodejs:mainfrom
addaleax:sprintf-string-view-format-arg
Closed

src: allow std::string_view arguments to SPrintF() and friends#60058
addaleax wants to merge 1 commit into
nodejs:mainfrom
addaleax:sprintf-string-view-format-arg

Conversation

@addaleax

Copy link
Copy Markdown
Member

Modernize the code so there is no need to work with raw C strings anymore.

@nodejs-github-botnodejs-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 Sep 28, 2025
@addaleax
addaleaxforce-pushed the sprintf-string-view-format-arg branch from 84646ef to 88a83f1CompareSeptember 28, 2025 12:44
Modernize the code so there is no need to work with raw C strings
anymore.
@addaleax
addaleaxforce-pushed the sprintf-string-view-format-arg branch from 88a83f1 to 5a68e33CompareSeptember 28, 2025 12:48
@codecov

codecovBot commented Sep 28, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 59.25926% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.45%. Comparing base (c6316f9) to head (5a68e33).
⚠️ Report is 13 commits behind head on main.

Files with missing linesPatch %Lines
src/debug_utils-inl.h54.16%5 Missing and 6 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #60058 +/- ##
==========================================
- Coverage 88.45% 88.45% -0.01% 
==========================================
Files 703 703 Lines 207546 207710 +164 Branches 40011 40012 +1 ==========================================
+ Hits 183591 183729 +138 - Misses 15949 15973 +24 - Partials 8006 8008 +2 
Files with missing linesCoverage Δ
src/debug_utils.h80.00% <ø> (ø)
src/node_errors.h87.17% <100.00%> (-0.33%)⬇️
src/debug_utils-inl.h83.78% <54.16%> (-2.07%)⬇️

... and 60 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.

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 28, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

addaleax added a commit that referenced this pull request Sep 30, 2025
Modernize the code so there is no need to work with raw C strings
anymore.
PR-URL: #60058
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@addaleax

Copy link
Copy Markdown
MemberAuthor

Landed in 3b0b6b9

targos pushed a commit that referenced this pull request Oct 6, 2025
Modernize the code so there is no need to work with raw C strings
anymore.
PR-URL: #60058
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
aduh95 pushed a commit that referenced this pull request Oct 22, 2025
Modernize the code so there is no need to work with raw C strings
anymore.
PR-URL: #60058
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs 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++.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@addaleax@nodejs-github-bot@jasnell@legendecas@juanarbol