Skip to content
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: make Obs.combineLatest consistent with Flowable + doc cornercase #4987

Merged
merged 2 commits into from
Jan 12, 2017

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