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

Implemented combineLatest semantics #207

Merged
merged 5 commits into from
Mar 27, 2013
Merged

Conversation

jmhofer
Copy link
Contributor

@jmhofer jmhofer commented Mar 25, 2013

... at least if I understand them correctly...

This is an attempt at resolving #29

@cloudbees-pull-request-builder

RxJava-pull-requests #49 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #52 SUCCESS
This pull request looks good

@cloudbees-pull-request-builder

RxJava-pull-requests #53 SUCCESS
This pull request looks good

@benjchristensen
Copy link
Member

This looks good and fixes the behavior. Thank you @jmhofer for first finding this issue and then tackling the solution as I imagine it wasn't fun code to dig into.

The only thing I would want to look at changing (and it originates from my original code that you were modifying) is try to move away from blocking/synchronization towards non-blocking/atomics if possible to reduce contention while still retaining the atomicity required for the contract. But that is an optimization that may or may not warrant the effort and certainly does not need to hold up accepting this code as is to fix the fundamental problem.

benjchristensen added a commit that referenced this pull request Mar 27, 2013
Implemented combineLatest semantics
@benjchristensen benjchristensen merged commit 77c7011 into ReactiveX:master Mar 27, 2013
rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 2014
Implemented combineLatest semantics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants