Skip to content

buffer: add end parameter - #62390

Merged
nodejs-github-bot merged 1 commit into
mainfrom
buffer-index-of-end
Apr 11, 2026
Merged

buffer: add end parameter#62390
nodejs-github-bot merged 1 commit into
mainfrom
buffer-index-of-end

Conversation

@ronag

Copy link
Copy Markdown
Member

To limit how far into the buffer we search without allocating an unnecessary subarray.

@ronag
ronag requested review from jasnell and mcollinaMarch 22, 2026 09:54
@ronagronag self-assigned this Mar 22, 2026
@nodejs-github-botnodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Mar 22, 2026
@ronag
ronagforce-pushed the buffer-index-of-end branch 2 times, most recently from 6dc1d1e to f622fffCompareMarch 22, 2026 09:55
@ronag
ronagforce-pushed the buffer-index-of-end branch from f622fff to b57fa1fCompareMarch 22, 2026 09:58
ronag added a commit that referenced this pull request Mar 22, 2026
To limit how far into the buffer we search without allocating an
unnecessary subarray.
PR-URL: #62390
@ronag
ronagforce-pushed the buffer-index-of-end branch from b57fa1f to b739413CompareMarch 22, 2026 10:08
@ronag
ronag marked this pull request as ready for review March 22, 2026 10:08
ronag added a commit that referenced this pull request Mar 22, 2026
To limit how far into the buffer we search without allocating an
unnecessary subarray.
PR-URL: #62390
@ronag
ronagforce-pushed the buffer-index-of-end branch from b739413 to 5690750CompareMarch 22, 2026 10:39

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@mcollina

Copy link
Copy Markdown
Member

CI does not seem happy

ronag added a commit that referenced this pull request Mar 22, 2026
To limit how far into the buffer we search without allocating an
unnecessary subarray.
PR-URL: #62390
@ronag
ronagforce-pushed the buffer-index-of-end branch from 5690750 to ddbc9cdCompareMarch 22, 2026 13:36
@ronagronag added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Mar 22, 2026
@codecov

codecovBot commented Mar 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.13861% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.80%. Comparing base (bf1aebc) to head (fd6b746).
⚠️ Report is 214 commits behind head on main.

Files with missing linesPatch %Lines
src/node_buffer.cc70.83%4 Missing and 10 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #62390 +/- ##
==========================================
+ Coverage 89.72% 89.80% +0.08% 
==========================================
Files 695 699 +4 Lines 214464 216422 +1958 Branches 41067 41384 +317 ==========================================
+ Hits 192420 194360 +1940 - Misses 14106 14146 +40 + Partials 7938 7916 -22 
Files with missing linesCoverage Δ
lib/buffer.js99.18% <100.00%> (+0.01%)⬆️
src/node_buffer.cc68.03% <70.83%> (-0.17%)⬇️

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

@ronag
ronag requested a review from mcollinaMarch 22, 2026 16:00
Comment threaddoc/api/buffer.md Outdated
```

### `buf.includes(value[, byteOffset][, encoding])`
### `buf.includes(value[, byteOffset[, end]][, encoding])`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When we start getting into new optional arguments in the middle like this I think we should consider an alternative signature with an options bag instead...

buf.includes(value,{byteOffset: 0,end: 10,encoding: 'utf8',});

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

follow up PR?

@github-actionsgithub-actionsBot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Failed to start CI
 ⚠ Commits were pushed since the last approving review:
⚠ - buffer: add end parameter
✘ Refusing to run CI on potentially unsafe PR
https://github.com/nodejs/node/actions/runs/23542726197

@aduh95

Copy link
Copy Markdown
Contributor

There's a conflict to solve

@ronag
ronagforce-pushed the buffer-index-of-end branch from ddbc9cd to c4aecb4CompareApril 4, 2026 18:07
@ronagronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 11, 2026
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 11, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/62390
✔ Done loading data for nodejs/node/pull/62390
----------------------------------- PR info ------------------------------------
Title buffer: add end parameter (#62390)
Author Robert Nagy <ronagy@icloud.com> (@ronag)
Branch ronag:buffer-index-of-end -> nodejs:main
Labels buffer, c++, semver-minor, author ready, needs-ci, review wanted
Commits 1
- buffer: add end parameter
Committers 1
- Robert Nagy <ronagy@icloud.com>
PR-URL: https://github.com/nodejs/node/pull/62390
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/62390
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
--------------------------------------------------------------------------------
ℹ This PR was created on Sun, 22 Mar 2026 09:53:58 GMT
✔ Approvals: 2
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/62390#pullrequestreview-4067847074
✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/62390#pullrequestreview-4092335147
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2026-04-11T10:37:41Z: https://ci.nodejs.org/job/node-test-pull-request/72624/
⚠ Commits were pushed after the last Full PR CI run:
⚠ - buffer: add end parameter
- Querying data for job/node-test-pull-request/72624/
✔ Build data downloaded
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/24286987028

@ronagronag added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Apr 11, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 11, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronagronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 11, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 11, 2026
@nodejs-github-bot
nodejs-github-bot merged commit f2fda60 into mainApr 11, 2026
75 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in f2fda60

@nodejs-github-bot
nodejs-github-bot deleted the buffer-index-of-end branch April 11, 2026 19:24
@ChALkeR

Copy link
Copy Markdown
Member

Security: This should not land on a release without a fix for #62873

@ChALkeR

ChALkeR commented Apr 21, 2026

Copy link
Copy Markdown
Member

Adding dont-land-on-* as a temporary precaution, can be removed once #62873 is fixed.

Upd: dont-land-on-* can be removed once this PR lands:

@ChALkeRChALkeR added dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. labels Apr 21, 2026
@ronag

Copy link
Copy Markdown
MemberAuthor

Fixed by #62711

@panvapanva added the dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. label Apr 24, 2026
@panvapanva added baking-for-lts PRs that need to wait before landing in a LTS release. and removed dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. labels Apr 24, 2026
@ChALkeRChALkeR removed dont-land-on-v20.x dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Apr 24, 2026
@aduh95aduh95 mentioned this pull request May 5, 2026
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.bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.review wantedPRs that need reviews.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@ronag@mcollina@aduh95@nodejs-github-bot@ChALkeR@jasnell@panva@Renegade334@avivkeller