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

doc/api: clarify the documentation of pipes and zlib objects in them #22354

Closed
wants to merge 3 commits into from
Closed

doc/api: clarify the documentation of pipes and zlib objects in them #22354

wants to merge 3 commits into from

Conversation

andreasg123
Copy link
Contributor

@andreasg123 andreasg123 commented Aug 16, 2018

I made a first attempt to change the documentation for #22341. I don't know why stream.Transform didn't turn into a link in zlib.md. Somebody else would need to fix that if you accept this PR.

Checklist

Document how pipes can be chained in readable.pipe().
Document that zlib.Zlib inherits from stream.Transform.

Fixes: #22341
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 16, 2018
@gdams
Copy link
Member

gdams commented Aug 16, 2018

@@ -397,6 +397,9 @@ added: v0.5.8
Not exported by the `zlib` module. It is documented here because it is the base
class of the compressor/decompressor classes.

This class inherits from [`stream.Transform`][], allowing `zlib` objects to be

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Reference-style links were new to me.

@vsemozhetbyt vsemozhetbyt added stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem. labels Aug 16, 2018
@vsemozhetbyt
Copy link
Contributor

cc @nodejs/zlib @nodejs/streams

@vsemozhetbyt

This comment has been minimized.

@vsemozhetbyt
Copy link
Contributor

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

* Returns: {stream.Writable} making it possible to set up chains of piped
streams
* Returns: {stream.Writable} The *destination*, allowing for a chain of pipes if
it is a [`Duplex`][] stream
Copy link
Member

Choose a reason for hiding this comment

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

I would add “or a Transform” - a reader might not know that Transform inherits from
Duplex.

@vsemozhetbyt
Copy link
Contributor

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 17, 2018
@vsemozhetbyt vsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Aug 18, 2018
@vsemozhetbyt
Copy link
Contributor

Landed in 44d04a8
Thank you!

vsemozhetbyt pushed a commit that referenced this pull request Aug 18, 2018
Document how pipes can be chained in readable.pipe().
Document that zlib.Zlib inherits from stream.Transform.

PR-URL: #22354
Fixes: #22341
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Aug 19, 2018
Document how pipes can be chained in readable.pipe().
Document that zlib.Zlib inherits from stream.Transform.

PR-URL: #22354
Fixes: #22341
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@andreasg123 andreasg123 deleted the doc-stream branch August 20, 2018 18:02
targos pushed a commit that referenced this pull request Sep 3, 2018
Document how pipes can be chained in readable.pipe().
Document that zlib.Zlib inherits from stream.Transform.

PR-URL: #22354
Fixes: #22341
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
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. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land. stream Issues and PRs related to the stream subsystem. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants