-
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
Reimplement the timeout operator and fix timeout bugs #851
Conversation
Sorry that I have no time to update the java docs tonight. I'll finish it tomorrow. |
RxJava-pull-requests #774 SUCCESS |
*/ | ||
public final class OperatorTimeout<T> extends OperatorTimeoutBase<T> { | ||
|
||
public OperatorTimeout(final long timeout, final TimeUnit timeUnit, |
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.
You know you don't need to wrap at 80 characters right? :-)
We all have very wide-screen monitors now!
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 will update my IDE settings. Is "140 characters" OK?
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'd like to hear a consensus on this too... I've been vacillating between
the 80-column Sun javadoc recommendation and the 100-column Netflix
standard but don't have any money on any particular horse. Although
screens are wide these days, IDE panels often aren't. Github seems to like
to truncate lines at 112 columns, at least in my browser, FWIW.
On Wed, Feb 12, 2014 at 3:54 AM, Shixiong Zhu [email protected]:
In rxjava-core/src/main/java/rx/operators/OperatorTimeout.java:
+import rx.Observable;
+import rx.Scheduler;
+import rx.Scheduler.Inner;
+import rx.Subscription;
+import rx.util.functions.Action1;
+
+/**
- * Applies a timeout policy for each element in the observable sequence, using
- * the specified scheduler to run timeout timers. If the next element isn't
- * received within the specified timeout duration starting from its predecessor,
- * the other observable sequence is used to produce future messages from that
- * point on.
- */
+public final class OperatorTimeout extends OperatorTimeoutBase {
+- public OperatorTimeout(final long timeout, final TimeUnit timeUnit,
I will update my IDE settings. Is "140 characters" OK?
—
Reply to this email directly or view it on GitHubhttps://github.com//pull/851/files#r9664539
.
David M. Gross
PLP Consulting
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'm fine with 110-140 ... I'm even fine with it being longer if it feels more natural.
Most lines will be shorter than the 112 limit of Github, but most code is in an IDE where about 130-140 is normal on screens now.
In other words, if every single line was 140, it would be overwhelming, but when most are going to be far shorter, occasional lines > 110 are fine and it's better to avoid awkward wrapping. I turned off the auto-format on my IDE as sometimes it was annoying to have a line wrapped just because it's 1 character over.
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.
Thanks for your advise. Sometimes, I may work on my laptop. So I prefer 112. I will update the setting to 112.
I haven't had time to go through line-by-line and fully understand, but it looks clean and the tests suggest it's good. Thank you, this looks like it took some work! |
Reimplement the timeout operator and fix timeout bugs
This PR reimplemented the
timeout
operator. I also fixed the following bugs oftimeout
:timeout
can not work on a synchronized Observable. Unit test:zsxwing@356a690#diff-f181dce6b07869d0daacb404a292ce0cR231
TimeoutException
when a timeout event happens without an other Observable. Unit test: zsxwing@356a690#diff-836fc7028db94329e793dcc6075a258dR259and
zsxwing@356a690#diff-836fc7028db94329e793dcc6075a258dR289
onNext
andtimeout
methods. Unit test: zsxwing@356a690#diff-836fc7028db94329e793dcc6075a258dR321