Closed indutny closed 7 years ago
As far as I can see, this would tremendously help native addons writers also, since StreamBase is indeed very bulky. Decreasing boilerplate one has to implement to hook in C programs into js-streams is definitely worth an addition like this. Awesome work, btw.
Indeed, this looks great! The more we can leverage libuv to do the heavy lifting generically, the better. :smile_cat:
oooo... yes please. this definitely looks promising.
Added note about observability and data
events. This is very important too.
Overall I dig it. Would like @bnoordhuis' opinion.
@trevnorris thank you!
Thank you for review @trevnorris !
The document should provide some detail on how uv_link_t is supposed to work. It currently just, ah, links to https://github.com/indutny/uv_link_t without further explanation.
@bnoordhuis I can copy-paste "How?" from https://github.com/indutny/uv_link_t#how , will it suffice?
Yes, but it should probably also say a few words about how it's going to be hooked up in node.js.
@bnoordhuis sorry for delay, pushed update.
Should I land this? Its been awhile since the last review, and I believe that there is a general agreement about this feature.
Should I land this?
Since it says status is DRAFT, and the README says all EPs regardless of status should be landed...I'd say yes. Landing it as DRAFT status especially should be uncontroversial.
Alright, landing!
Landed in 2c45bf0, thank you!
cc @nodejs/ctc @trevnorris @nodejs/collaborators @saghul