Skip to content

2.x: make Obs.combineLatest consistent with Flowable + doc cornercase#4987

Merged
akarnokd merged 2 commits intoReactiveX:2.xfrom
akarnokd:ObsCombineLatestStop
Jan 12, 2017
Merged

2.x: make Obs.combineLatest consistent with Flowable + doc cornercase#4987
akarnokd merged 2 commits intoReactiveX:2.xfrom
akarnokd:ObsCombineLatestStop

Conversation

@akarnokd
Copy link
Member

@akarnokd akarnokd commented Jan 12, 2017

This PR fixes Observable.combineLatest to be consistent with Flowable.combineLatest by not subscribing to additional input sources if the operator reached a terminal state due to a valueless source (that completes or errors). In addition, such early termination didn't properly cancel the other sources when delayErrors == true.

I've also extended the documentation on the overloads to warn about empty sources that will terminate the operator, even with combineLatestDelayError, and thus subscription side-effects may not happen.

There is a related issue #4414 where the operator should fully consume each input source no matter what and terminate when all terminate. I'm still considering what would be the best way to introduce this.

Reported in #4986

@codecov-io
Copy link

codecov-io commented Jan 12, 2017

Current coverage is 95.62% (diff: 100%)

Merging #4987 into 2.x will decrease coverage by <.01%

@@                2.x      #4987   diff @@
==========================================
  Files           592        592          
  Lines         37968      37968          
  Methods           0          0          
  Messages          0          0          
  Branches       5752       5752          
==========================================
- Hits          36307      36306     -1   
  Misses          701        701          
- Partials        960        961     +1   

Powered by Codecov. Last update 5717827...4ea2a05

@akarnokd akarnokd merged commit d1cd153 into ReactiveX:2.x Jan 12, 2017
@akarnokd akarnokd deleted the ObsCombineLatestStop branch January 12, 2017 18:25
@ferhatparmak
Copy link

ferhatparmak commented Jan 14, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants