stream: add compose operator #44937
stream: add compose operator #44937
Conversation
|
Review requested:
|
|
Thanks for reviewing
I don't think so, it's still a valuable api
I did not add a documentation yet cause @ronag said that this may not be merged, so I'll wait for merge approval and then I'll write the docs 😀 |
|
@ronag @benjamingr @jasnell should I keep developing it? I don't wanna waste my time in case we decide not to do it 😊 |
|
Hey sorry, just getting on top of things and will review. |
|
Added validation, aside from the docs I think this PR is ready It would be great if someone could help me with the docs |
github-actions
bot
removed
the
request-ci
doc/api/stream.md
Outdated
|
|
||
|
|
||
|
|
||
| See [`stream.compose`][] for more information. |
|
What to do about the failing lint? it fails cause the line is too long, but the line is long because of the link which is not displayed... |
github-actions
bot
removed
the
request-ci
github-actions
bot
removed
the
request-ci
benjamingr
added
the
commit-queue
nodejs-github-bot
added
commit-queue-failed
Commit Queue failed- Loading data for nodejs/node/pull/44937 ✔ Done loading data for nodejs/node/pull/44937 ----------------------------------- PR info ------------------------------------ Title stream: add compose operator (#44937) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch rluvaton:feat/add-compose-stream-operator -> nodejs:main Labels needs-ci Commits 7 - stream: add compose operator - stream: rename import - stream: add abort signal support - stream: add validation and refactor tests - stream: fix lint and add started adding docs - stream: added an example in the streams docs - stream: fix lint error Committers 1 - Raz Luvaton <16746759+rluvaton@users.noreply.github.com> PR-URL: https://github.com/nodejs/node/pull/44937 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Robert Nagy ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/44937 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Robert Nagy -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 09 Oct 2022 10:50:07 GMT ✔ Approvals: 3 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/44937#pullrequestreview-1158053238 ✔ - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/44937#pullrequestreview-1161868722 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/44937#pullrequestreview-1159123210 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2022-10-31T11:41:05Z: https://ci.nodejs.org/job/node-test-pull-request/47589/ - Querying data for job/node-test-pull-request/47589/ ✔ 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 44937 From https://github.com/nodejs/node * branch refs/pull/44937/merge -> FETCH_HEAD ✔ Fetched commits as ffa2e964e882..7e55077b63a6 -------------------------------------------------------------------------------- [main 91eaa5aa1f] stream: add compose operator Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 13:43:49 2022 +0300 2 files changed, 98 insertions(+) create mode 100644 test/parallel/test-stream-compose-operator.js [main 18e34bf99c] stream: rename import Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 13:46:14 2022 +0300 1 file changed, 2 insertions(+), 2 deletions(-) [main 12370446d3] stream: add abort signal support Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun Oct 9 16:04:37 2022 +0300 2 files changed, 55 insertions(+), 11 deletions(-) [main 52e55a1bb8] stream: add validation and refactor tests Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed Oct 26 22:02:02 2022 +0300 3 files changed, 30 insertions(+), 16 deletions(-) Auto-merging doc/api/stream.md [main b9d363ceb1] stream: fix lint and add started adding docs Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Wed Oct 26 22:21:10 2022 +0300 3 files changed, 24 insertions(+), 5 deletions(-) Auto-merging doc/api/stream.md [main 261e4c014b] stream: added an example in the streams docs Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Fri Oct 28 01:39:24 2022 +0300 1 file changed, 20 insertions(+), 2 deletions(-) Auto-merging doc/api/stream.md [main bcd6bd6930] stream: fix lint error Author: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sat Oct 29 23:09:44 2022 +0300 1 file changed, 4 insertions(+), 2 deletions(-) ✔ Patches applied There are 7 commits in the PR. Attempting autorebase. Rebasing (2/14)https://github.com/nodejs/node/actions/runs/3361645437 |
benjamingr
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in ddb3ae7 |
PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
PR-URL: nodejs#44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
PR-URL: #44937 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
Start adding compose function #43664
I've started implementing from scratch and then realized that I don't need to, Using the
composefunction is enough.The only problem I have is with the abort signal as the static
composedoesn't seem to support it so I used theaddAbortSignalbut we won't pass the stream the signal as the rest of the operatorsCc @benjamingr