FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

stream: add isErrored helper by ronag · Pull Request #41121 · nodejs/node · GitHub

/ node Public

stream: add isErrored helper - #41121

Closed
ronag wants to merge 6 commits into
nodejs:masterfrom
nxtedition:stream-is-errored
ConversationCommits (6)ChecksFiles changed
Closed

stream: add isErrored helper#41121
ronag wants to merge 6 commits into
nodejs:masterfrom
nxtedition:stream-is-errored

Conversation

ronag commented Dec 9, 2021

Copy link
Copy Markdown
Member

ronag added stream Issues and PRs related to the stream subsystem. web streams labels Dec 9, 2021
ronag requested review from jasnell and mcollina December 9, 2021 08:16
nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 9, 2021
ronag force-pushed the stream-is-errored branch from eeab6d4 to 46092fb Compare December 9, 2021 08:16

ronag commented Dec 9, 2021

Copy link
Copy Markdown
Member Author

@nodejs/streams

ronag mentioned this pull request Dec 9, 2021
Merged
ronag force-pushed the stream-is-errored branch from 46092fb to 9c79a04 Compare December 9, 2021 08:18
mcollina approved these changes Dec 9, 2021
View reviewed changes

mcollina 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

ronag force-pushed the stream-is-errored branch 2 times, most recently from c76beb4 to 1e2413b Compare December 9, 2021 09:33
ronag force-pushed the stream-is-errored branch from 1e2413b to 7ade6e3 Compare December 9, 2021 09:47
ronag added author ready PRs that have at least one approval, no outstanding review comments, 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 Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2021

nodejs-github-bot commented Dec 9, 2021

Copy link
Copy Markdown
Collaborator

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

lpinca reviewed Dec 9, 2021
View reviewed changes
Comment thread doc/api/stream.md Outdated
mscdex reviewed Dec 9, 2021
View reviewed changes
Comment thread doc/api/stream.md Outdated
mscdex reviewed Dec 9, 2021
View reviewed changes
Comment thread doc/api/stream.md Outdated

mscdex commented Dec 9, 2021

Copy link
Copy Markdown
Contributor

Is there a reason this is only for Readable streams?

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

ronag commented Dec 9, 2021
View reviewed changes
Comment thread doc/api/stream.md Outdated
ronag and others added 2 commits December 9, 2021 15:34
Apply suggestions from code review
47b2605 
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
Update doc/api/stream.md
571739e 
Co-authored-by: mscdex <mscdex@users.noreply.github.com>
ronag commented Dec 9, 2021
View reviewed changes
Comment thread doc/api/stream.md Outdated
ronag requested review from lpinca and mscdex December 9, 2021 14:34
ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 14, 2021
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 commented Dec 14, 2021

Copy link
Copy Markdown
Collaborator

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

This was referenced Dec 15, 2021
Open
Open
ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 16, 2021
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 commented Dec 16, 2021

Copy link
Copy Markdown
Collaborator

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

ronag mentioned this pull request Dec 16, 2021
Closed

nodejs-github-bot commented Dec 16, 2021

Copy link
Copy Markdown
Collaborator

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

ronag commented Dec 16, 2021

Copy link
Copy Markdown
Member Author

Landed in 752d75d

ronag added a commit that referenced this pull request Dec 16, 2021
stream: add isErrored helper
752d75d 
Refs: nodejs/undici#1134

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

lpinca commented Dec 16, 2021

Copy link
Copy Markdown
Member

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
stream: add isErrored helper
9b0cafd 
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
github-actions Bot mentioned this pull request Dec 17, 2021
Open
22 tasks
danielleadams pushed a commit that referenced this pull request Dec 17, 2021
stream: add isErrored helper
b59c513 
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
This was referenced Dec 18, 2021
Open
Open
Open
Open
Open
danielleadams pushed a commit that referenced this pull request Jan 31, 2022
stream: add isErrored helper
2568ac9 
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 Feb 1, 2022
stream: add isErrored helper
4522216 
Refs: nodejs/undici#1134

PR-URL: #41121
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
danielleadams mentioned this pull request Feb 1, 2022
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mcollina mcollina approved these changes
JungMinu JungMinu approved these changes
jasnell Awaiting requested review from jasnell
lpinca Awaiting requested review from lpinca
mscdex Awaiting requested review from mscdex
+1 more reviewer
aymen94 aymen94 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. stream Issues and PRs related to the stream subsystem. web streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

Back | FazBrowse Home | New Git URL