search

yutakahirano / fetch-with-streams

Fetch API integrated with Streams.
66 stars 8 forks source link

Add a link to the stream spec for body teeing. #33

Closed yutakahirano closed 9 years ago

yutakahirano commented 9 years ago

This fixes #14. @domenic, can you take a look?

domenic commented 9 years ago

LGTM. Minor nits: I would do

Let «out1, out2» be TeeReadableStream(the associated readable stream, true)

Justification: TeeReadableStream returns a list, which uses «» instead of [] (which is for arrays), and in #28 I use the function-call-style syntax for abstract ops so using it here to match seems nice.

annevk commented 9 years ago

However, Fetch currently does not use such syntax...

domenic commented 9 years ago

I mean, Fetch doesn't have abstract operations. In the end it's an integration point between two specs, so you have to pick a calling convention and stick with it.

annevk commented 9 years ago

Fair, hopefully we can converge on something over time, but I guess once I attempt the merge with Fetch I can call out the differences in notes while we're in a transitional time.

domenic commented 9 years ago

Yeah, I guess I just don't really see a problem with using another spec's notation when calling into another spec, even if that notation is different from what is used internally.

yutakahirano commented 9 years ago

Thanks, fixed.