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: Note that zlib.flush acts after pending writes #6172

Closed

addaleax wants to merge 2 commits into nodejs:master from addaleax:doc-zlib-flush-after-pending-writes

Member

addaleax commented Apr 12, 2016

Checklist

documentation is changed or added
the commit message follows commit guidelines

Affected core subsystem(s)

doc

Description of change

Describe that zlib.flush() may wait for pending writes. This was discussed a bit in #3782; and although it’s not really related to the original issue report there, the addition probably gives off the right impression, i.e. that flush() is not as immediate as it may sound.

mscdex added doc zlib labels

Member

jasnell commented Apr 13, 2016

LGTM

Member Author

addaleax commented Apr 17, 2016

@jasnell I’d like to land this if your lgtm meant that this resolves #3782 for you :)

Member

jasnell commented Apr 18, 2016

@addaleax ... I think it partially does at least. The situation that I describe in #3782 still isn't entirely clear with this addition. An example included in the doc could likely help also but that can be added in a separate PR.

Member Author

addaleax commented Apr 18, 2016

@jasnell Nah, I’m happy to add stuff to this PR.
Is there anything from #3782 (comment) you’d like to have included? I mean one could explicitly write something along the lines of .flush() respecting the highWaterMark, but that would seem to say “This stream is like all other readable streams”.

Member Author

addaleax commented Apr 18, 2016

Also, 👍 on that an example for .flush() would be nice. I guess I’ll get to that tomorrow or so.


          doc: Note that zlib.flush acts after pending writes

5a84875

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

addaleax force-pushed the doc-zlib-flush-after-pending-writes branch from 61615ca to 5a84875 Compare

April 18, 2016 17:00

Member Author

addaleax commented Apr 18, 2016

Updated this with a bit more explanation that’s specific to the situation in #3782.

Member

jasnell commented Apr 18, 2016

@addaleax +1 .. that change works for me! if there's a concise example that can illustrate the point, then awesome. If not, I'm good with this landing as is.

Member Author

addaleax commented Apr 18, 2016

I’ll sleep over that, but maybe it’s just best noted as a comment in a generic .flush() example.

Contributor

stevemao commented Apr 19, 2016

@nodejs/documentation

Contributor

eljefedelrodeodeljefe commented Apr 19, 2016

Example would be nice if you get one together. In any case LGTM.


          doc: add full example for zlib.flush()

b81e3e2

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

Member Author

addaleax commented Apr 19, 2016

@eljefedelrodeodeljefe Was literally finishing a first take at an example when you wrote that 😄 Would love to hear what you think of it!

Contributor

eljefedelrodeodeljefe commented Apr 19, 2016

Thought so. Being in the same timezone leads to concurrent working apparently. :) I am a little foreign to zlib code, but I think that illustrates it very well. 👍

Member Author

addaleax commented Apr 19, 2016

@eljefedelrodeodeljefe Keep in mind that people who are foreign to the specifics are precisely the target groups of the docs, so any questions or suggestions are greatly appreciated! :)

Member Author

addaleax commented Apr 20, 2016

@jasnell Any thoughts on the second commit/the example? Are you fine with having this merged + marked as a fix for #3782? :)

Member

jasnell commented Apr 20, 2016

I like it. Thank you very much. LGTM

addaleax added a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

9c2b8ec

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

addaleax added a commit that referenced this pull request


          doc: add full example for zlib.flush()

5f0fcd6

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

Member Author

addaleax commented Apr 20, 2016

Landed in 5f0fcd6 and 9c2b8ec. :-)

addaleax closed this

addaleax deleted the doc-zlib-flush-after-pending-writes branch

April 20, 2016 16:32

addaleax added the lts-watch-v4.x label

MylesBorins pushed a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

50f3f10

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: add full example for zlib.flush()

e6f4a17

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

This was referenced Apr 21, 2016

V5.11.0 proposal #6321

Closed

V5.11.0 proposal #6322

Closed

MylesBorins pushed a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

9f9371b

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: add full example for zlib.flush()

02a84e2

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

joelostrowski pushed a commit to joelostrowski/node that referenced this pull request


          doc: note that zlib.flush acts after pending writes

c3eb8fa

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: nodejs#3782
PR-URL: nodejs#6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

joelostrowski pushed a commit to joelostrowski/node that referenced this pull request


          doc: add full example for zlib.flush()

ac9d9e9

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: nodejs#6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

jasnell pushed a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

eb92eaa

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

jasnell pushed a commit that referenced this pull request


          doc: add full example for zlib.flush()

d94283c

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins added land-on-v4.x and removed lts-watch-v4.x labels

MylesBorins pushed a commit that referenced this pull request


          doc: add full example for zlib.flush()

3188cea

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

34aca08

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: add full example for zlib.flush()

d069f2d

Add a full example using `zlib.flush()` for the common use
case of writing partial compressed HTTP output to the client.

PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins pushed a commit that referenced this pull request


          doc: note that zlib.flush acts after pending writes

Describe that `zlib.flush()` may wait for pending writes and
until output is being read from the stream.

Fixes: #3782
PR-URL: #6172
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins mentioned this pull request

v4.4.5 proposal #6824

Merged

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

Labels

doc zlib