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.

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

doc: the method readable.push description #29687

Closed
wants to merge 1 commit into from
Closed

doc: the method readable.push description #29687

wants to merge 1 commit into from

Conversation

ImHype
Copy link
Contributor

@ImHype ImHype commented Sep 24, 2019

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. stream Issues and PRs related to the stream subsystem. labels Sep 24, 2019
@ImHype ImHype changed the title doc: the method description doc: the method readable.push description Sep 24, 2019
@ImHype
Copy link
Contributor Author

ImHype commented Sep 24, 2019

#29680

@ImHype ImHype force-pushed the doc_29680 branch 6 times, most recently from c3d30c8 to 65eb07e Compare September 24, 2019 07:00
@reergymerej
Copy link

This is a clearer description.

Trott
Trott reviewed Sep 25, 2019
View reviewed changes
doc/api/stream.md Outdated Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Sep 25, 2019

@nodejs/streams @nodejs/documentation

jasnell
jasnell approved these changes Sep 25, 2019
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Just a nit: lines need to be wrapped at <= 80 chars

@ImHype
Copy link
Contributor Author

ImHype commented Sep 26, 2019 •
edited
Loading

Fine, I got it. Thanks for your approve.

mcollina
mcollina approved these changes Sep 27, 2019
View reviewed changes
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

@mcollina
Copy link
Member

CI lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3937/

@trivikr
Copy link
Member

trivikr commented Sep 27, 2019

CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3938/

@trivikr trivikr added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 27, 2019
@Trott
Copy link
Member

Trott commented Sep 27, 2019

Landed in 63c0f15

Trott pushed a commit that referenced this pull request Sep 27, 2019
@ImHype @Trott
doc: clarify description of readable.push() method
63c0f15 
PR-URL: #29687
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@Trott Trott closed this Sep 27, 2019
@lpinca lpinca mentioned this pull request Sep 28, 2019
Closed
targos pushed a commit that referenced this pull request Oct 1, 2019
@ImHype @targos
doc: clarify description of readable.push() method
1303e35 
PR-URL: #29687
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@targos targos mentioned this pull request Oct 1, 2019
Merged
@beevelop beevelop mentioned this pull request Oct 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott left review comments

@trivikr trivikr trivikr approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

@jasnell jasnell jasnell approved these changes

@mcollina mcollina mcollina approved these changes

Assignees
No one assigned
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. stream Issues and PRs related to the stream subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ImHype @reergymerej @Trott @mcollina @trivikr @jasnell @BethGriggs @nodejs-github-bot