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

Reimplement the timeout operator and fix timeout bugs #851

Merged
merged 4 commits into from
Feb 11, 2014

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Feb 11, 2014

This PR reimplemented the timeout operator. I also fixed the following bugs of timeout:

@zsxwing
Copy link
Member Author

zsxwing commented Feb 11, 2014

Sorry that I have no time to update the java docs tonight. I'll finish it tomorrow.

@cloudbees-pull-request-builder

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

*/
public final class OperatorTimeout<T> extends OperatorTimeoutBase<T> {

public OperatorTimeout(final long timeout, final TimeUnit timeUnit,
Copy link
Member

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!

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 will update my IDE settings. Is "140 characters" OK?

Copy link
Collaborator

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 notifications@github.comwrote:

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

Copy link
Member

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.

Copy link
Member Author

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.

@benjchristensen
Copy link
Member

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!

benjchristensen added a commit that referenced this pull request Feb 11, 2014
Reimplement the timeout operator and fix timeout bugs
@benjchristensen benjchristensen merged commit 3d3894f into ReactiveX:master Feb 11, 2014
@zsxwing zsxwing deleted the timeout branch March 2, 2014 03:20
jihoonson pushed a commit to jihoonson/RxJava that referenced this pull request Mar 6, 2020
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

4 participants