Removing streams and importing from dojo-streams, issue #225

dojo / core

:rocket: Dojo 2 - language helpers and utilities.

http://dojo.io

Other

213 stars 62 forks source link

Removing streams and importing from dojo-streams, issue #225 #239

Closed rorticus closed 7 years ago

rorticus commented 7 years ago

Moving streams into a separate package,

https://github.com/rorticus/streams

I'm not really sure how to make rorticus/streams into dojo/streams but I'm guessing someone smart can tell me!

codecov-io commented 7 years ago

Current coverage is 94.56% (diff: 100%)

Merging #239 into master will increase coverage by 0.13%

@@             master       #239   diff @@
==========================================
  Files            48         31    -17   
  Lines          2461       1638   -823   
  Methods          28         17    -11   
  Messages          0          0          
  Branches        465        330   -135   
==========================================
- Hits           2324       1549   -775   
+ Misses           51         28    -23   
+ Partials         86         61    -25

Powered by Codecov. Last update 0b3a513...99d84af

dylans commented 7 years ago

I'm not really sure how to make rorticus/streams into dojo/streams but I'm guessing someone smart can tell me!

Basically someone with admin in both accounts can move a repo, and then you can refork it once it has been moved.

rorticus commented 7 years ago

I believe we were hoping to also refactor request/node.ts to not depend on streams

I can try that! Would that be using observables instead?

dylans commented 7 years ago

Would that be using observables instead?

I'm not sure I know the right answer to that. To be fair, streams within Node.js usage is probably a good thing, but it feels wrong for all of core to rely on streams with the only dependency being the Node.js request API. Perhaps take a look at how it's being used and make a suggestion?

rorticus commented 7 years ago

OK, so... it looks like it's not being used... I did a search for streamTarget (the name of the option that gets passed to a node request to use a stream) on all the dojo repositories and the only hits are in the node request file...

dylans commented 7 years ago

Right, the only thing that would eventually use it would be a Node.js request (which was to be supported for crypto and io, which have been deferred as we've not been focusing on Node.js APIs, though we want what we have to work inside Node.js... hopefully that distinction makes sense).