Skip to content

assert: remove dead code #58760

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
Merged
merged 3 commits into from
Jun 29, 2025
Merged

assert: remove dead code #58760

merged 3 commits into from
Jun 29, 2025
+16 −25

Conversation

kt3k
Copy link
Contributor

@kt3k kt3k commented Jun 19, 2025

code variable in getErrMessage util is not initialized anymore since this commit 43c380e and the handling of it is now dead code. This PR removes them.

@nodejs-github-bot nodejs-github-bot added assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run. labels Jun 19, 2025
kt3k
kt3k commented Jun 19, 2025
View reviewed changes
lib/internal/assert/utils.js
@@ -199,7 +198,7 @@ function getErrMessage(message, fn) {
// errors are handled faster.
if (errorStackTraceLimitIsWritable) Error.stackTraceLimit = 0;

if (filename) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is if (filename) { ... } else { return ... } structure above, and this check is unnecessary.

@kt3k kt3k force-pushed the remove-dead-code branch from 74fee45 to b1563bd Compare June 19, 2025 04:35
@kt3k kt3k changed the title refactor(assert): remove dead code assert: remove dead code Jun 19, 2025
@codecov Codecov
Copy link

codecov bot commented Jun 19, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.10%. Comparing base (214e4db) to head (b09eb11).
Report is 73 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #58760      +/-   ##
==========================================
- Coverage   90.13%   90.10%   -0.04%     
==========================================
  Files         639      640       +1     
  Lines      188192   188278      +86     
  Branches    36911    36917       +6     
==========================================
+ Hits       169634   169648      +14     
- Misses      11287    11338      +51     
- Partials     7271     7292      +21
Files with missing lines Coverage Δ
lib/internal/assert/utils.js 92.08% <100.00%> (+2.19%) ⬆️

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

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 19, 2025
@nodejs-github-bot
Copy link
Collaborator

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

Ethan-Arrowood
Ethan-Arrowood reviewed Jun 20, 2025
View reviewed changes
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 26, 2025
@nodejs-github-bot
Copy link
Collaborator

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

@gurgunday gurgunday added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. and removed needs-ci PRs that need a full CI run. labels Jun 26, 2025
BridgeAR
BridgeAR approved these changes Jun 26, 2025
View reviewed changes
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 28, 2025 •
edited by jasnell
Loading

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

@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jun 29, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 29, 2025
@nodejs-github-bot nodejs-github-bot merged commit 7db30d7 into nodejs:main Jun 29, 2025
76 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 7db30d7

targos pushed a commit that referenced this pull request Jul 3, 2025
@kt3k @targos
assert: remove dead code
a118bfc 
PR-URL: #58760
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
@github-actions github-actions bot mentioned this pull request Jul 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lpinca lpinca

@anonrig anonrig

@legendecas legendecas

@BridgeAR BridgeAR

@Ethan-Arrowood Ethan-Arrowood

@marco-ippolito marco-ippolito

Assignees
No one assigned
Labels
assert Issues and PRs related to the assert subsystem. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. 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
@kt3k @nodejs-github-bot @lpinca @anonrig @legendecas @BridgeAR @Ethan-Arrowood @marco-ippolito @jasnell @gurgunday