-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2.x: fix Observable.concatMapEager bad logic for immediate scalars #4982
Conversation
.concatMapEager(new Function<Integer, Flowable<Integer>>() { | ||
@Override | ||
public Flowable<Integer> apply(Integer i) throws Exception { | ||
System.out.println("Processing " + i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just cluttering it up and can be removed?
.concatMapEager(new Function<Integer, ObservableSource<Integer>>() { | ||
@Override | ||
public ObservableSource<Integer> apply(Integer i) throws Exception { | ||
System.out.println("Processing " + i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Current coverage is 95.65% (diff: 100%)@@ 2.x #4982 diff @@
==========================================
Files 592 592
Lines 37977 37968 -9
Methods 0 0
Messages 0 0
Branches 5754 5752 -2
==========================================
+ Hits 36287 36320 +33
+ Misses 720 700 -20
+ Partials 970 948 -22
|
The operator
Observable.concatMapEager
had a bad optimization targeting scalar and callable sources and emitted their values immediately even if it wasn't that particular source's turn for it.The
Flowable
is not affected, added unit tests for both.Reported in #4981.