-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Implement ReplaySubject with infinite history #218
Conversation
RxJava-pull-requests #64 FAILURE |
RxJava-pull-requests #65 FAILURE |
Conflicts: rxjava-core/src/main/java/rx/operators/OperationTake.java
RxJava-pull-requests #66 SUCCESS |
I'm unavailable this week but will review this when I get back. I was focusing on getting schedulers done last week and wasn't able to get to this one. |
import rx.util.functions.Func1; | ||
|
||
public class Subject<T> extends Observable<T> implements Observer<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This guts the Subject
class which should exist as it's own thing. See .Net for the equivalent: http://msdn.microsoft.com/en-us/library/system.reactive.subjects.subject(v=vs.103).aspx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then what do you suggest for the name of RxJava's analogue for Rx's ISubject
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And you should see that I renamed Subject
to PublishSubject
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the dilemma is ISubject vs Subject ... so what becomes what ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I see that @johngmyers ... I'm trying to figure out how to merge this and a couple other pull requests that all started overlapping. Apparently everyone wanted the same things at the same time :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning on doing ConnectableObservable
after getting feedback on my approach so far. The other pull request doesn't deal with subjects other than the publish Subject
. I would suggest you decline that other pull request until after you've figured out how you want to deal with Rx's ISubject
.
Please weigh in on this: #242 |
I would appreciate some feedback on UnsubscribeTester. I left some comments in the commits above. Note there were todo comments asking about desired semantics in the added unit tests. |
No description provided.