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.

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

buffer: properly apply dst offset and src length on fast path #54391

Merged

Conversation

ronag
Copy link
Member

@ronag ronag commented Aug 15, 2024

No description provided.

@ronag ronag added the buffer Issues and PRs related to the buffer subsystem. label Aug 15, 2024
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 15, 2024
@ronag
Copy link
Member Author

ronag commented Aug 15, 2024

@targos @anonrig Is there a way to force our tests to call fast API? Not sure why our existing tests didn't catch this. Nor how to make them catch this.

@ronag ronag changed the title buffer: Properly apply offset on fast path buffer: properly apply offset on fast path Aug 15, 2024
@ronag

This comment was marked as resolved.

ronag added a commit to nxtedition/node that referenced this pull request Aug 15, 2024
@ronag ronag force-pushed the fast-string-missing-offset branch from e708a48 to 41c2d89 Compare August 15, 2024 06:20
ronag added a commit to nxtedition/node that referenced this pull request Aug 15, 2024
@ronag ronag force-pushed the fast-string-missing-offset branch 2 times, most recently from f0ba68b to ea45c6c Compare August 15, 2024 06:25
ronag added a commit to nxtedition/node that referenced this pull request Aug 15, 2024
ronag added a commit to nxtedition/node that referenced this pull request Aug 15, 2024
@ronag ronag force-pushed the fast-string-missing-offset branch from ea45c6c to 9795353 Compare August 15, 2024 06:26
@ronag ronag force-pushed the fast-string-missing-offset branch from 9795353 to bbafef7 Compare August 15, 2024 06:28
@ronag ronag changed the title buffer: properly apply offset on fast path buffer: properly apply dst offset and src length on fast path Aug 15, 2024
@ronag ronag added the fast-track PRs that do not need to wait for 48 hours to land. label Aug 15, 2024
Copy link
Contributor

Fast-track has been requested by @ronag. Please 👍 to approve.

@ronag ronag requested a review from benjamingr August 15, 2024 06:29
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 15, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.08%. Comparing base (ccf05ef) to head (bbafef7).
Report is 391 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54391      +/-   ##
==========================================
+ Coverage   86.33%   87.08%   +0.74%     
==========================================
  Files         648      648              
  Lines      182290   182310      +20     
  Branches    34812    34985     +173     
==========================================
+ Hits       157385   158757    +1372     
+ Misses      18213    16822    -1391     
- Partials     6692     6731      +39     
Files with missing lines Coverage Δ
src/node_buffer.cc 70.64% <100.00%> (+0.03%) ⬆️

... and 98 files with indirect coverage changes

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 15, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 15, 2024
@nodejs-github-bot nodejs-github-bot merged commit 2c14615 into nodejs:main Aug 15, 2024
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 2c14615

RafaelGSS pushed a commit that referenced this pull request Aug 19, 2024
Refs: #54311 (comment)
PR-URL: #54391
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Aug 19, 2024
RafaelGSS pushed a commit that referenced this pull request Aug 21, 2024
Refs: #54311 (comment)
PR-URL: #54391
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Jake Yuesong Li <[email protected]>
@targos targos added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants