GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — src: disable fast methods for `buffer.write` by targos · Pull Request #54565 · nodejs/node

Conversation

targos commented Aug 26, 2024

Copy link

Copy Markdown

Member

It should resolve the regressions while we work on fixing them.

Refs: #54521

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss, olsendevs, ihmpavel, bozolia, and loremaps reacted with rocket emoji

ihmpavel reacted with eyes emoji

All reactions

🚀 5 reactions
👀 1 reaction


          src: disable fast methods for buffer.write

a7a10a7

It should resolve the regressions while we work on fixing them.

Refs: nodejs#54521

nodejs-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

Aug 26, 2024

targos mentioned this pull request

Aug 26, 2024

v22.8.0 proposal #54560

Merged

codecov Bot commented Aug 26, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.33%. Comparing base (1399d4e) to head (a7a10a7).
Report is 323 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #54565      +/-   ##
==========================================
- Coverage   87.33%   87.33%   -0.01%     
==========================================
  Files         649      649              
  Lines      182626   182620       -6     
  Branches    35041    35040       -1     
==========================================
- Hits       159503   159489      -14     
- Misses      16394    16404      +10     
+ Partials     6729     6727       -2

Files with missing lines	Coverage Δ
src/node_buffer.cc	`69.94% <100.00%> (-0.71%)`	⬇️

... and 23 files with indirect coverage changes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos commented Aug 26, 2024

Copy link

Copy Markdown

Member Author

If this is accepted, it should be fast-tracked for inclusion in #54560

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos added the fast-track PRs proposed for a shorter-than-standard waiting period before landing. label

Aug 26, 2024

github-actions Bot commented Aug 26, 2024

Copy link

Copy Markdown

Contributor

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

bricss, benjamingr, ronag, lpinca, anonrig, and addaleax reacted with thumbs up emoji

All reactions

👍 6 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr commented Aug 26, 2024

Copy link

Copy Markdown

Member

@bricss please do not approve fast-track requests :] (or you know, start making PRs and join as a collaborator that also works :))

bricss reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr approved these changes

Aug 26, 2024

View reviewed changes

ronag approved these changes

Aug 26, 2024

View reviewed changes

targos added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 26, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 26, 2024

nodejs-github-bot commented Aug 26, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca approved these changes

Aug 26, 2024

View reviewed changes

anonrig approved these changes

Aug 26, 2024

View reviewed changes

ronag added author ready

PRs with CI started, the required approvals, and no outstanding review comments.

request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels

Aug 26, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Aug 26, 2024

nodejs-github-bot commented Aug 26, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addaleax approved these changes

Aug 26, 2024

View reviewed changes

nodejs-github-bot commented Aug 26, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

avivkeller mentioned this pull request

Aug 26, 2024

SWC + next.js + Node.js v22.7 #54532

Closed

nodejs-github-bot commented Aug 27, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ronag commented Aug 27, 2024

Copy link

Copy Markdown

Member

@targos can you fix commit msg (PR-URL)?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

avivkeller mentioned this pull request

Aug 27, 2024

UTF+8 encodings are broken #54543

Closed

jasnell approved these changes

Aug 27, 2024

View reviewed changes

jasnell reviewed

Aug 27, 2024

View reviewed changes

Comment thread

src/node_buffer.cc

		"utf8WriteStatic",
		SlowWriteString<UTF8>,
		&fast_write_string);
		SetMethod(context, target, "asciiWriteStatic", SlowWriteString<ASCII>);

jasnell Aug 27, 2024

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

non-blocking nit: perhaps consider adding a TODO comment?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

olsendevs reacted with eyes emoji

All reactions

👀 1 reaction

targos commented Aug 27, 2024

Copy link

Copy Markdown

Member Author

@ronag I don't see a problem in the commit message.

avivkeller and olsendevs reacted with eyes emoji

All reactions

👀 2 reactions

Commit message must match a given regex pattern: PR-URL: https://github\.com/(nodejs|nodejs-private)/(node|node-private)/pull/\d+

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos commented Aug 28, 2024

Copy link

Copy Markdown

Member Author

This message appears (to collaborators) on all the pull requests of the project. Unfortunately there is no way to customize it but the goal is to prevent merging by accident using the GitHub UI.

avivkeller and addaleax reacted with thumbs up emoji

All reactions

👍 2 reactions

All reactions

🚀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca added the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 29, 2024

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 29, 2024

nodejs-github-bot merged commit 7616855 into nodejs:main

Aug 29, 2024

nodejs-github-bot commented Aug 29, 2024

Copy link

Copy Markdown

Collaborator

Landed in 7616855

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos deleted the buffer-fast-write-disable branch

August 29, 2024 07:24

avivkeller mentioned this pull request

Aug 29, 2024

Buffer.from() output breaks after optimizing in nodejs 22.7 #54521

Closed

RafaelGSS pushed a commit that referenced this pull request

Aug 30, 2024


          src: disable fast methods for buffer.write

e071651

It should resolve the regressions while we work on fixing them.

Refs: #54521
PR-URL: #54565
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

emilazy mentioned this pull request

Sep 3, 2024

nodejs_22: 22.6.0 -> 22.7.0 NixOS/nixpkgs#336556

Merged

2 of 13 tasks

trentm added a commit to trentm/opentelemetry-js that referenced this pull request

Sep 4, 2024


          chore: unpin unit-test from 22.6

3d69d6b

Now that Node.js 22.8 is available with a workaround
(nodejs/node#54565) for the bug in 22.7 that
caused 'RangeError: "length" is outside of buffer bounds' errors in
exporter tests (see open-telemetry#4953),
we can unpin the Node.js v22 used for unit tests.
This undoes the pinning from open-telemetry#4957.

trentm mentioned this pull request

Sep 4, 2024

chore: unpin unit-test from 22.6 open-telemetry/opentelemetry-js#4977

Merged

targos added the dont-land-on-v20.x label

Sep 22, 2024

Reviewers

jasnell

jasnell approved these changes

addaleax

addaleax approved these changes

benjamingr

benjamingr approved these changes

lpinca

lpinca approved these changes

anonrig

anonrig approved these changes

ronag

ronag approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

buffer

Issues and PRs related to the buffer subsystem.

c++

Issues and PRs that require attention from people who are familiar with C++.

fast-track

PRs proposed for a shorter-than-standard waiting period before landing.

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.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

targos commented Aug 26, 2024

Uh oh!

codecov Bot commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

targos commented Aug 26, 2024

Uh oh!

github-actions Bot commented Aug 26, 2024

Uh oh!

benjamingr commented Aug 26, 2024

Uh oh!

nodejs-github-bot commented Aug 26, 2024

Uh oh!

nodejs-github-bot commented Aug 26, 2024

Uh oh!

nodejs-github-bot commented Aug 26, 2024

Uh oh!

nodejs-github-bot commented Aug 27, 2024

Uh oh!

ronag commented Aug 27, 2024

Uh oh!

jasnell Aug 27, 2024

Choose a reason for hiding this comment

Uh oh!

targos commented Aug 27, 2024

Uh oh!

nodejs-github-bot commented Aug 27, 2024

Uh oh!

nodejs-github-bot commented Aug 27, 2024

Uh oh!

nodejs-github-bot commented Aug 27, 2024

Uh oh!

nodejs-github-bot commented Aug 28, 2024

Uh oh!

ronag commented Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

targos commented Aug 28, 2024

Uh oh!

nodejs-github-bot commented Aug 28, 2024

Uh oh!

nodejs-github-bot commented Aug 28, 2024

Uh oh!

nodejs-github-bot commented Aug 28, 2024

Uh oh!

nodejs-github-bot commented Aug 29, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

codecov Bot commented Aug 26, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

ronag commented Aug 28, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.