Skip to content

Conversation

addaleax
Copy link
Member

If we can just use the classic THROW_...() methods directly, without needing to allocate an std::string for the message/format parameter, let's just do so.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Sep 28, 2025
If we can just use the classic `THROW_...()` methods directly,
without needing to allocate an `std::string` for the message/format
parameter, let's just do so.
@addaleax addaleax force-pushed the no-constant-std-string-format-arg-for-throw branch from 5d554db to 1440e19 Compare September 28, 2025 12:46
Copy link

codecov bot commented Sep 28, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.46%. Comparing base (c6316f9) to head (1440e19).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/node_file.cc 77.77% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #60057   +/-   ##
=======================================
  Coverage   88.45%   88.46%           
=======================================
  Files         703      703           
  Lines      207546   207526   -20     
  Branches    40011    40005    -6     
=======================================
- Hits       183591   183578   -13     
+ Misses      15949    15936   -13     
- Partials     8006     8012    +6     
Files with missing lines Coverage Δ
src/node_file.cc 75.64% <77.77%> (+0.04%) ⬆️

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

@addaleax addaleax 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-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

addaleax added a commit that referenced this pull request Sep 30, 2025
If we can just use the classic `THROW_...()` methods directly,
without needing to allocate an `std::string` for the message/format
parameter, let's just do so.

PR-URL: #60057
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
Member Author

Landed in 2e2f4cd

@addaleax addaleax closed this Sep 30, 2025
targos pushed a commit that referenced this pull request Oct 6, 2025
If we can just use the classic `THROW_...()` methods directly,
without needing to allocate an `std::string` for the message/format
parameter, let's just do so.

PR-URL: #60057
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 free to join this conversation on GitHub. Already have an account? Sign in to comment

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++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants