highland icon indicating copy to clipboard operation
highland copied to clipboard

Edit Laziness

Open o0x2a opened this issue 9 years ago • 6 comments

Function pull also cause a thunk.

o0x2a avatar Dec 07 '15 09:12 o0x2a

Thanks for the pull request, @code-guru! docs/index.html shouldn't be edited directly, as it's generated from the comments in lib/index.js. Could you update your pull request to change the template please? (Note that that section of the docs has changed slightly on master)

I'm a weak +1 for the change itself. It's technically correct, but I'm not sure we should be encouraging people to use pull (higher-level transforms can often replace it). @vqvu, what do you think?

apaleslimghost avatar Dec 07 '15 11:12 apaleslimghost

I'm also a weak +1, but for the reason that this sentence is not meant to be an exhaustive list of all methods that consume a stream---that's what the Consumption section is for.

pull's use is generally discouraged, but it is useful in for implementing higher-level transforms when the ones we already have are not adequate (e.g., @svozza's sortedMergeBy). It's use is not discouraged to the same level as pause and resume—I've never seen an external usecase for pause, and resume is only useful for changing the backpressure behavior, like in throttle. But since we already mention resume in that sentence, adding pull probably doesn't hurt our messaging any.

vqvu avatar Dec 07 '15 13:12 vqvu

It's not clear that the Consumption section is meant to be that exhaustive list, given that only some methods mention they cause a thunk (implying that the rest don't). Maybe we should add a short intro to that section, reiterating parts of Laziness.

apaleslimghost avatar Dec 07 '15 14:12 apaleslimghost

But since we already mention resume in that sentence, adding pull probably doesn't hurt our messaging any.

I guess the other way to go could be to remove resume from the sentence.

svozza avatar Dec 07 '15 14:12 svozza

It's not clear that the Consumption section is meant to be that exhaustive list, given that only some methods mention they cause a thunk (implying that the rest don't).

Well, the terminology that we use now is <em>consume</em> the stream, so a section named Consumption seems self-explanatory. pull says it "consumes a single item from the Stream" and pipe says it "pull[s] all the data from the source Highland Stream and write it to the destination". All of the methods mentions consuming the stream. So, I don't think it's confusing any longer.

I guess the other way to go could be to remove resume from the sentence.

I'd be OK with that. I'd also be OK with adding pull and taking out resume. pull is useful in certain circumstances, and we already have a warning in its entry saying "you probably don't need to use this".

vqvu avatar Dec 07 '15 16:12 vqvu

@code-guru, if you're still interested, can you update this PR to change the docs/templates/base.html file? You can also remove the part that mentions resume. Then we can merge this.

vqvu avatar Dec 16 '15 04:12 vqvu