groupBy not using lift #1085
Labels
help wanted
Issues we wouldn't mind assistance with.
Milestone
Comments
|
@Blesh I noticed this too. I'm working through a fix for |
|
I did that. At the time groupBy wasn't passing tests at all, so I just followed how RxJS 4 implemented it. |
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 4, 2016
Add test for groupBy() operator, to verify that it supports the composable lift architecture. Test for issue ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 4, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift()-based architecture in RxJS Next. Resolves ReactiveX#1085. BREAKING CHANGES: groupBy() now unsubscribes all inner Observables when the outer Observable is unsubscribed. This diverges from the behavior in RxJS 4, where inner Observables continue even if the outer is unsubscribed.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 13, 2016
Add test for groupBy() operator, to verify that it supports the composable lift architecture. Test for issue ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 13, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift()-based architecture in RxJS Next. Resolves ReactiveX#1085. BREAKING CHANGES: groupBy() now unsubscribes all inner Observables when the outer Observable is unsubscribed. This diverges from the behavior in RxJS 4, where inner Observables continue even if the outer is unsubscribed.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 18, 2016
Add test for groupBy() operator, to verify that it supports the composable lift architecture. Test for issue ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 18, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift()-based architecture in RxJS Next. Resolves ReactiveX#1085. BREAKING CHANGES: groupBy() now unsubscribes all inner Observables when the outer Observable is unsubscribed. This diverges from the behavior in RxJS 4, where inner Observables continue even if the outer is unsubscribed.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 18, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift-based architecture in RxJS Next Resolves ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 25, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift-based architecture in RxJS Next Resolves ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 26, 2016
Add test for groupBy() operator, to verify that it supports the composable lift architecture. Test for issue ReactiveX#1085.
staltz
added a commit
to staltz/RxJSNext
that referenced
this issue
Jan 26, 2016
Fix bug ReactiveX#1085 in which groupBy was not using lift(). Using lift() is critical to support the ubiquitous lift-based architecture in RxJS Next Resolves ReactiveX#1085.
benlesh
pushed a commit
that referenced
this issue
Jan 27, 2016
Add test for groupBy() operator, to verify that it supports the composable lift architecture. Test for issue #1085.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The current implementation of groupBy is no longer using
lift. This actually muck with some other plans in the works for logging and debugging hooks.GroupBy should be refactored to use
lift.If you look at it the
GroupByObservableis really just a factory for aSubscriber. That's what we useOperatorclass for. I clearly didn't review this one very well when I merged it a long time ago. It seems thatRefCountSubscriptioninstance could be folded into the operator's subscriber as well.The text was updated successfully, but these errors were encountered: