Skip to content

docs,test: add tests and docs for duplex.fromWeb and duplex.toWeb#42738

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.

Sign up for GitHub

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

Jump to bottom
Merged
nodejs-github-bot merged 5 commits into from
Apr 28, 2022
Merged

docs,test: add tests and docs for duplex.fromWeb and duplex.toWeb #42738

nodejs-github-bot merged 5 commits into from
Apr 28, 2022

Conversation

@ErickWendel
Copy link
Member

@ErickWendelErickWendel commented Apr 14, 2022

Improve Code coverage for Duplex.fromWeb and Duplex.toWeb

This also adds examples of how to use Duplex.fromWeb and Duplex.toWeb

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 14, 2022
@ErickWendelErickWendelforce-pushed the erick/improve-code-coverage-for-duplex-web branch 2 times, most recently from 9e6359a to 9ea3dcaCompareApril 14, 2022 14:00
@ErickWendelErickWendel changed the title stream/duplex: add tests and docs for duplex.fromWeb and duplex.toWebstream: add tests and docs for duplex.fromWeb and duplex.toWebApr 14, 2022
@ErickWendelErickWendelforce-pushed the erick/improve-code-coverage-for-duplex-web branch from 9ea3dca to 0d50b71CompareApril 14, 2022 14:17
@mscdex
Copy link
Contributor

mscdex commented Apr 14, 2022

I think the subsystem prefix on the commit message would better be represented by doc,test instead of stream.

@ErickWendelErickWendel changed the title stream: add tests and docs for duplex.fromWeb and duplex.toWebdocs,test: add tests and docs for duplex.fromWeb and duplex.toWebApr 14, 2022
@ErickWendelErickWendelforce-pushed the erick/improve-code-coverage-for-duplex-web branch from 0d50b71 to 3a1c6aeCompareApril 14, 2022 14:48
@ErickWendel
Copy link
MemberAuthor

ErickWendel commented Apr 14, 2022

I think the subsystem prefix on the commit message would better be represented by doc,test instead of stream.

I liked it! Just changed both commit message and PR name

lpinca
lpinca approved these changes Apr 14, 2022
View reviewed changes
Mesteery
Mesteery approved these changes Apr 14, 2022
View reviewed changes
@ErickWendel
all small nits suggestions
d248554
Signed-off-by: Erick Wendel <[email protected]>
@Trott
Copy link
Member

Trott commented Apr 15, 2022

Should there be CJS examples too so that people have the toggle switch in the docs? It looks like we haven't been doing that in stream.md but you can look at lots of other docs (like crypto.md) for examples and see what it looks like on the website:

image

@ErickWendel
Copy link
MemberAuthor

ErickWendel commented Apr 18, 2022

Should there be CJS examples too so that people have the toggle switch in the docs? It looks like we haven't been doing that in stream.md but you can look at lots of other docs (like crypto.md) for examples and see what it looks like on the website:

image

Nice! I didn't know that. Just added it!

@TrottTrott 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. labels Apr 18, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 18, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 19, 2022

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

aduh95
aduh95 reviewed Apr 19, 2022
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

Choose a reason for hiding this comment

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

Can you please add the node: prefix to align with #42752?

ErickWendeland others added 2 commits April 19, 2022 21:10
@ErickWendel@aduh95
Update doc/api/stream.md
d5d117e
Co-authored-by: Antoine du Hamel <[email protected]>
@ErickWendel@aduh95
docs: apply suggestions from code review
6e2967e
Co-authored-by: Antoine du Hamel <[email protected]>
@ErickWendel
Copy link
MemberAuthor

ErickWendel commented Apr 20, 2022

Can you please add the node: prefix to align with #42752?

done!

@github-actionsgithub-actionsbot mentioned this pull request Apr 20, 2022
Open
16 tasks
@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2022
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 20, 2022

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 20, 2022

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 20, 2022

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

This was referenced Apr 21, 2022
Open
Open
Open
Open
Open
Open
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 26, 2022

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 27, 2022

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 27, 2022

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

@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 28, 2022
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 28, 2022
@nodejs-github-botnodejs-github-bot merged commit bc47eb3 into nodejs:masterApr 28, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 28, 2022

Landed in bc47eb3

targos pushed a commit that referenced this pull request May 2, 2022
@ErickWendel@targos
doc,test: add tests and docs for duplex.fromWeb and duplex.toWeb
a6e1e7a
PR-URL: #42738 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Mestery <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
@targostargos mentioned this pull request May 2, 2022
Merged
@juanarbol
Copy link
Member

juanarbol commented May 31, 2022

Depending of #39134 being backported to v16.x

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@lpincalpincalpinca approved these changes

@aduh95aduh95aduh95 approved these changes

@juanarboljuanarboljuanarbol approved these changes

+1 more reviewer

@MesteeryMesteeryMesteery approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.needs-ciPRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@ErickWendel@mscdex@Trott@nodejs-github-bot@juanarbol@jasnell@lpinca@aduh95@Mesteery@targos