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

stream: add isErrored helper #41121

Closed
wants to merge 6 commits into from
Closed

Conversation

ronag
Copy link
Member

@ronag ronag commented Dec 9, 2021

@ronag ronag added stream Issues and PRs related to the stream subsystem. web streams labels Dec 9, 2021
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 9, 2021
@ronag
Copy link
Member Author

ronag commented Dec 9, 2021

@nodejs/streams

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@ronag ronag force-pushed the stream-is-errored branch 2 times, most recently from c76beb4 to 1e2413b Compare December 9, 2021 09:33
@ronag ronag added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed needs-ci PRs that need a full CI run. labels Dec 9, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2021
@nodejs-github-bot
Copy link
Collaborator

doc/api/stream.md Outdated Show resolved Hide resolved
doc/api/stream.md Outdated Show resolved Hide resolved
doc/api/stream.md Outdated Show resolved Hide resolved
@mscdex
Copy link
Contributor

mscdex commented Dec 9, 2021

Is there a reason this is only for Readable streams?

Also, maybe we should name it hasErrored() or hasError() instead?

doc/api/stream.md Outdated Show resolved Hide resolved
ronag and others added 2 commits December 9, 2021 15:34
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
doc/api/stream.md Outdated Show resolved Hide resolved
@ronag ronag requested review from lpinca and mscdex December 9, 2021 14:34
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2021
@nodejs-github-bot
Copy link
Collaborator

@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 16, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 16, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@ronag
Copy link
Member Author

ronag commented Dec 16, 2021

Landed in 752d75d

ronag added a commit that referenced this pull request Dec 16, 2021
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@ronag ronag closed this Dec 16, 2021
@ronag ronag mentioned this pull request Dec 16, 2021
@lpinca
Copy link
Member

lpinca commented Dec 16, 2021

It's a bit too late but I agree with @mscdex about the name. hasErrored() seems more correct.

danielleadams pushed a commit that referenced this pull request Dec 16, 2021
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 17, 2021
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 31, 2022
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
Refs: nodejs/undici#1134

PR-URL: nodejs#41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. stream Issues and PRs related to the stream subsystem. web streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants