zebulonj / callbag-subscribe

A callbag sink (listener) that connects an Observer a-la RxJS. 👜
MIT License
18 stars 5 forks source link

Add support for callback as argument and merge with callbag-observe #5

Closed zimme closed 6 years ago

zimme commented 6 years ago

Could we update the API of this package to also match the API of callbag-observe and then have callbag-observe be a proxy for callbag-subscribe or is there a reason for having them as separate packages with slightly different implementations?

/cc @stalz

zimme commented 6 years ago

Also, isn't this pretty much the same as callbag-for-each but with a slightly different api.

staltz commented 6 years ago

Yes that's correct

zimme commented 6 years ago

Possible implementation here #6.

zebulonj commented 6 years ago

@zimme Unless callbag-for-each has changed since I wrote this... it's pretty different in that callbag-for-each didn't handle the termination of a callbag and didn't support disposal.

zimme commented 6 years ago

My thinking here was that callbag-subscribe with the proposed changes in #6 would allow callbag-observe and callbag-foreach just be umbrella packages that brings in this package instead.

Just a thought.

zimme commented 6 years ago

But maybe callbag-for-each needs the behaviour of requiring a callback function, which this package doesn't.

However I don't see this need for callbag-subscribe so maybe at least those could be "merged" into this package.

zebulonj commented 6 years ago

Yeah... just refreshed my memory of callbag-observe, which also doesn't allow for explicit handling of termination of the callbag (either complete or error) and doesn't provide for disposal.

I'm not opposed to the thought of consolidation. If the change looks sound, I'll merge it. Then it will depend on whether @staltz wants to "proxy" to callbag-subscribe. But... considering I originally offered to implement this functionality there and he declined, I'm guessing that will be a no.

zimme commented 6 years ago

Well, just a thought. But as he said in that issue, there's no "official" callbag packages so choose what fits the bill or write your own as it's quite easy =)

...and I chose this package :+1:

zebulonj commented 6 years ago

Merged #6.