2.x: Rename Observable Base Interface Types for consistency #4300
Conversation
| @@ -17,6 +17,7 @@ | |||
| import java.util.concurrent.Callable; | |||
|
|
|||
| import io.reactivex.*; | |||
| import io.reactivex.ObservableSource; | |||
There was a problem hiding this comment.
There's superfluous imports like this throughout this PR and your other one, but I don't think it's a big deal.
There was a problem hiding this comment.
Yup IntelliJ is not smart here and automatically inserts the import. Could also be because I've configured it to avoid * imports ...
Once I'm done with all of the renaming I could fix all of the imports if this is wanted.
|
There are a large amount of compilation errors. |
| @@ -18,9 +18,9 @@ | |||
| import io.reactivex.internal.disposables.DisposableHelper; | |||
| import io.reactivex.plugins.RxJavaPlugins; | |||
|
|
|||
| public final class ObservableDematerialize<T> extends ObservableSource<Try<Optional<T>>, T> { | |||
| public final class ObservableDematerialize<T> extends ObservableSource { | |||
There was a problem hiding this comment.
Looks like there already was an ObservableSource class that we're now clashing with. I'm tempted to just inline it since all it does is hold a value in a field. As an actual type in the hierarchy it's not used in any special way. Either that or just rename it ObservableFromUpstream for now and delete it later.
There was a problem hiding this comment.
It's probably worth doing these as separate PRs. Rename/delete the existing ObservableSource and then do this rename. Otherwise it's likely to be extra confusing because of the name conflict.
There was a problem hiding this comment.
The original ObservableSource has its use by mandating a constructor with an upstream source and may later support some graph discovery by providing a standard way of getting the upstream.
There was a problem hiding this comment.
Makes sense. Does ObservableFromUpstream or ObservableWithUpstream for that sound good?
There was a problem hiding this comment.
ObservableWithUpstream is okay.
There was a problem hiding this comment.
Created #4031 to do this.
|
Rebased against 2.x and re-did the last changes. |
|
looks good this time 👍 |
Current coverage is 69.42% (diff: 98.00%)@@ 2.x #4300 diff @@
==========================================
Files 420 420
Lines 30762 30761 -1
Methods 0 0
Messages 0 0
Branches 4937 4937
==========================================
- Hits 21358 21357 -1
- Misses 7319 7322 +3
+ Partials 2085 2082 -3
|
|
👍 |
Addresses #4044.