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

Add replay and multicast variants to RxScala #1160

Merged
merged 8 commits into from
May 20, 2014

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented May 6, 2014

This PR has some breaking changes:

  • Change def multicast[R >: T](subject: rx.lang.scala.Subject[R]): (() => Subscription, Observable[R]) to def multicast[R >: T](subject: rx.lang.scala.Subject[R]): ConnectableObservable[R]
  • Change def replay: (() => Subscription, Observable[T]) to def replay: ConnectableObservable[T]
  • Remove Observable.apply(Int)

So it's better to merge it at the next major release.
#1152, #1153.

/cc @headinthebox @samuelgruetter

@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

I'd like someone with better Scala skills that me to review this please.

@headinthebox
Copy link
Contributor

I'll code review it later today, but in principle these are simple fixes to bring the other variants of publish in sync with returning connectable observable.

shared.subscribe(n => println(s"subscriber 1 gets $n"))
shared.subscribe(n => println(s"subscriber 2 gets $n"))
shared.connect
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example does not really enlighten users on what multicast is good for, because they just see the same output as if they hadn't used it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this example from exampleWithPublish since publish = multicast(Subject()).

  @Test def exampleWithPublish() {
    val unshared = Observable.from(1 to 4)
    val shared = unshared.publish
    shared.subscribe(n => println(s"subscriber 1 gets $n"))
    shared.subscribe(n => println(s"subscriber 2 gets $n"))
    shared.connect
  }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I got it, we can l leave it, because the output is different

@zsxwing zsxwing mentioned this pull request May 7, 2014
Conflicts:
	language-adaptors/rxjava-scala/src/examples/scala/rx/lang/scala/examples/RxScalaDemo.scala
@cloudbees-pull-request-builder

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

Conflicts:
	language-adaptors/rxjava-scala/src/examples/scala/rx/lang/scala/examples/RxScalaDemo.scala
@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

Is this reviewed and ready to go?

It is conflicting against master, so can you please rebase?

@zsxwing
Copy link
Member Author

zsxwing commented May 16, 2014

This is also conflicting against #1178. I would like to rebase it after #1178 is merged to master.

@zsxwing
Copy link
Member Author

zsxwing commented May 20, 2014

Waiting until #1210 is merged.

Conflicts:
	language-adaptors/rxjava-scala/src/examples/scala/rx/lang/scala/examples/RxScalaDemo.scala
	language-adaptors/rxjava-scala/src/main/scala/rx/lang/scala/Observable.scala
	language-adaptors/rxjava-scala/src/test/scala/rx/lang/scala/CompletenessTest.scala
@zsxwing
Copy link
Member Author

zsxwing commented May 20, 2014

Already merged

@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

Should this wait until we're ready to do 0.19?

@zsxwing
Copy link
Member Author

zsxwing commented May 20, 2014

I think this one can be merged since #1188 has already broken the API.

@headinthebox @samuelgruetter , do you mind it?

@zsxwing zsxwing mentioned this pull request May 20, 2014
@headinthebox
Copy link
Contributor

Go for it.

benjchristensen added a commit that referenced this pull request May 20, 2014
Add `replay` and `multicast` variants to RxScala
@benjchristensen benjchristensen merged commit 1c76782 into ReactiveX:master May 20, 2014
@zsxwing zsxwing deleted the replay-multicast branch May 22, 2014 15:21
@benjchristensen benjchristensen mentioned this pull request Jun 1, 2014
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.

None yet

5 participants