2.x: Single & Completable have implicit Flowable subscribe methods but Observable does not

[JakeWharton]

There's Single.subscribe(Subscription) and Completable.subscribe(Subscription) but no Observable.subscribe(Subscription). Feels like we should either remove the two defined methods and require explicit conversions or add the missing method to Observable.

I'm in favor of removal, but I don't know if there's additional story as to why they're present.

[akarnokd]

It allows testing Single and Compleatable with TestSubscriber. I didn't add Observable.subscribe(Subscriber) to avoid overload resolution problems in case someone implements both ˙Subscriber and Observer.

[artem-zinnatullin]

Yup, noticed that too, but figured out there are both TestSubscriber and
TestObserver again (I was under impression that we're removing TestObserver
after changes in 1.x)

On Fri, Aug 19, 2016 at 9:55 AM, David Karnok notifications@github.com
wrote:

It allows testing Single and Compleatable with TestSubscriber. I didn't
add Observable.subscribe(Subscriber) to avoid overload resolution
problems in case someone implements both ˙Subscriber and Observer.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#4373 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA7B3J5E5MwIrrPUgyMzV2lesgDVkjrdks5qhVNigaJpZM4JoKZ2
.

[JakeWharton]

Can we just add .test() methods to Single and Completable (if they're not
already there) to eliminate the subscribe() overloads then?

On Fri, Aug 19, 2016 at 2:57 AM Artem Zinnatullin notifications@github.com
wrote:

Yup, noticed that too, but figured out there are both TestSubscriber and
TestObserver again (I was under impression that we're removing TestObserver
after changes in 1.x)

On Fri, Aug 19, 2016 at 9:55 AM, David Karnok notifications@github.com
wrote:

It allows testing Single and Compleatable with TestSubscriber. I didn't
add Observable.subscribe(Subscriber) to avoid overload resolution
problems in case someone implements both ˙Subscriber and Observer.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#4373 (comment)
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AA7B3J5E5MwIrrPUgyMzV2lesgDVkjrdks5qhVNigaJpZM4JoKZ2

.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4373 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEERYqHuj6cAz3p00rAxTh7bKayKZDks5qhVPBgaJpZM4JoKZ2
.

[akarnokd]

#4375 adds those methods so yes.

[JakeWharton]

I saw that. Great! Does that also mean we can remove the methods? Or do you
want them for some other purpose?

On Fri, Aug 19, 2016 at 11:07 AM David Karnok notifications@github.com
wrote:

#4375 #4375 adds those methods
so yes.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4373 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEERiYGGTGqMctw7nv73ix2toiq5xFks5qhcadgaJpZM4JoKZ2
.

[akarnokd]

They are implemented like toFlowable().subscribe() anyway so I can live with that. So you can remove them. Also while you are at it, cosider renaming Subject.hasSubscribers to Subject.hasObservers.

[JakeWharton]

Will do after lunch!

On Fri, Aug 19, 2016 at 11:15 AM David Karnok notifications@github.com
wrote:

They are implemented like toFlowable().subscribe() anyway so I can live
with that. So you can remove them. Also while you are at it, cosider
renaming Subject.hasSubscribers to Subject.hasObservers.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4373 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEEEeIaEzel26JwN-UL1fGALKBAJW4cks5qhciSgaJpZM4JoKZ2
.

[akarnokd]

I've started a PR myself with other changes so I'll make these as well.

[akarnokd]

See #4378.

[akarnokd]

Closing via #4375 and #4378