stream: validate writable defaultEncoding by marco-ippolito · Pull Request #46322 · nodejs/node

marco-ippolito · 2023-01-23T21:15:25Z

fixes: #46301
this has to be done for stream.Readable too for consistency. I'll open another pr for readable if this gets approved

nodejs-github-bot · 2023-01-23T21:15:29Z

Review requested:

@nodejs/streams

mcollina

What happens if encoding is set and objectMode is set?

marco-ippolito · 2023-01-24T09:08:33Z

What happens if encoding is set and objectMode is set?

in the example m.write({ foo: 'bar' }, 'hex'); ovverrides the default utf8 encoding.
I've added a test

mcollina

lgtm

ShogunPanda

LGTM!

nodejs-github-bot · 2023-01-29T17:23:54Z

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

nodejs-github-bot · 2023-01-30T09:48:18Z

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

nodejs-github-bot · 2023-01-30T16:05:40Z

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

nodejs-github-bot · 2023-01-30T17:39:43Z

Commit Queue failed

- Loading data for nodejs/node/pull/46322 ✔ Done loading data for nodejs/node/pull/46322 ----------------------------------- PR info ------------------------------------ Title stream: validate writable defaultEncoding (#46322) Author Marco Ippolito (@marco-ippolito) Branch marco-ippolito:feat/validate-default-encoding -> nodejs:main Labels semver-major Commits 4 - stream: validate writable defaultEncoding - stream: test falsy defaultEncoding - stream: test object mode - stream: refactor code style Committers 1 - Marco Ippolito PR-URL: https://github.com/nodejs/node/pull/46322 Fixes: https://github.com/nodejs/node/issues/46301 Reviewed-By: Robert Nagy Reviewed-By: Matteo Collina Reviewed-By: Paolo Insogna Reviewed-By: Benjamin Gruenbaum Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/46322 Fixes: https://github.com/nodejs/node/issues/46301 Reviewed-By: Robert Nagy Reviewed-By: Matteo Collina Reviewed-By: Paolo Insogna Reviewed-By: Benjamin Gruenbaum Reviewed-By: Yagiz Nizipli Reviewed-By: Luigi Pinca -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 23 Jan 2023 21:15:25 GMT ✔ Approvals: 6 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/46322#pullrequestreview-1266353265 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/46322#pullrequestreview-1267105299 ✔ - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/46322#pullrequestreview-1267507590 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/46322#pullrequestreview-1267936213 ✔ - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/46322#pullrequestreview-1270011746 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/46322#pullrequestreview-1270098666 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-01-30T16:05:40Z: https://ci.nodejs.org/job/node-test-pull-request/49252/ - Querying data for job/node-test-pull-request/49252/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 46322 From https://github.com/nodejs/node * branch refs/pull/46322/merge -> FETCH_HEAD ✔ Fetched commits as a5fd53f8eff7..958633dd2ad9 -------------------------------------------------------------------------------- [main ab718f9c95] stream: validate writable defaultEncoding Author: Marco Ippolito Date: Mon Jan 23 22:13:03 2023 +0100 2 files changed, 31 insertions(+), 1 deletion(-) [main 57e3f04abe] stream: test falsy defaultEncoding Author: Marco Ippolito Date: Tue Jan 24 09:32:37 2023 +0100 1 file changed, 13 insertions(+) [main eeb0cd9b48] stream: test object mode Author: Marco Ippolito Date: Tue Jan 24 10:07:00 2023 +0100 1 file changed, 10 insertions(+) [main 8842b7f615] stream: refactor code style Author: Marco Ippolito Date: Wed Jan 25 10:27:25 2023 +0100 1 file changed, 5 insertions(+), 4 deletions(-) ✔ Patches applied There are 4 commits in the PR. Attempting autorebase. Rebasing (2/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: validate writable defaultEncoding

PR-URL: #46322
Fixes: #46301
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD f2d9d32f1e] stream: validate writable defaultEncoding
Author: Marco Ippolito marcoippolito54@gmail.com
Date: Mon Jan 23 22:13:03 2023 +0100
2 files changed, 31 insertions(+), 1 deletion(-)
Rebasing (3/8)
Rebasing (4/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: test falsy defaultEncoding

PR-URL: #46322
Fixes: #46301
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD a06590b30f] stream: test falsy defaultEncoding
Author: Marco Ippolito marcoippolito54@gmail.com
Date: Tue Jan 24 09:32:37 2023 +0100
1 file changed, 13 insertions(+)
Rebasing (5/8)
Rebasing (6/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: test object mode

PR-URL: #46322
Fixes: #46301
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD a1ce83cf3d] stream: test object mode
Author: Marco Ippolito marcoippolito54@gmail.com
Date: Tue Jan 24 10:07:00 2023 +0100
1 file changed, 10 insertions(+)
Rebasing (7/8)
Rebasing (8/8)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
stream: refactor code style

PR-URL: #46322
Fixes: #46301
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Matteo Collina matteo.collina@gmail.com
Reviewed-By: Paolo Insogna paolo@cowtech.it
Reviewed-By: Benjamin Gruenbaum benjamingr@gmail.com
Reviewed-By: Yagiz Nizipli yagiz@nizipli.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com

[detached HEAD 3daaebfb85] stream: refactor code style
Author: Marco Ippolito marcoippolito54@gmail.com
Date: Wed Jan 25 10:27:25 2023 +0100
1 file changed, 5 insertions(+), 4 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/4046566650

nodejs-github-bot · 2023-01-30T17:54:31Z

Landed in 9e7093f

stream: validate writable defaultEncoding

68193ee

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 23, 2023

ronag added the semver-major PRs that contain breaking changes and should be released in the next major version. label Jan 23, 2023

ronag approved these changes Jan 23, 2023

View reviewed changes

anonrig reviewed Jan 24, 2023

View reviewed changes

Comment thread lib/internal/streams/writable.js Outdated

Comment thread test/parallel/test-stream-writable-decoded-encoding.js

Comment thread lib/internal/streams/writable.js Outdated

stream: test falsy defaultEncoding

f9a2d21

mcollina reviewed Jan 24, 2023

View reviewed changes

stream: test object mode

1b96251

mcollina approved these changes Jan 24, 2023

View reviewed changes

ShogunPanda approved these changes Jan 24, 2023

View reviewed changes

aduh95 reviewed Jan 24, 2023

View reviewed changes

Comment thread lib/internal/streams/writable.js Outdated

benjamingr approved these changes Jan 24, 2023

View reviewed changes

stream: refactor code style

958633d

marco-ippolito requested a review from aduh95 January 25, 2023 09:28

anonrig approved these changes Jan 25, 2023

View reviewed changes

anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023

lpinca approved these changes Jan 25, 2023

View reviewed changes

Comment thread test/parallel/test-stream-writable-decoded-encoding.js

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 25, 2023

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 26, 2023

jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 29, 2023

jasnell reviewed Jan 29, 2023

View reviewed changes

Comment thread lib/internal/streams/writable.js

ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 30, 2023

ShogunPanda added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Jan 30, 2023

nodejs-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 Jan 30, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 30, 2023

nodejs-github-bot merged commit 9e7093f into nodejs:main Jan 30, 2023

marco-ippolito mentioned this pull request Jan 30, 2023

stream: validate readable defaultEncoding #46430

Merged

RafaelGSS mentioned this pull request Apr 3, 2023

2023-04-18, Version 20.0.0 (Current) #47381

Merged

jasnell mentioned this pull request Oct 18, 2025

update node.js streams implementation cloudflare/workerd#5298

Merged

Uh oh!

Conversation

marco-ippolito commented Jan 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

marco-ippolito commented Jan 24, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 29, 2023

Uh oh!

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Successfully rebased and updated refs/heads/main.

Uh oh!

nodejs-github-bot commented Jan 30, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

marco-ippolito commented Jan 23, 2023 •

edited

Loading