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

1.x: Add a sentence to documentation of take() operator #5719

Merged
merged 2 commits into from
Nov 11, 2017
Merged

1.x: Add a sentence to documentation of take() operator #5719

merged 2 commits into from
Nov 11, 2017

Conversation

sadegh
Copy link
Contributor

@sadegh sadegh commented Nov 11, 2017

@codecov
Copy link

codecov bot commented Nov 11, 2017

Codecov Report

Merging #5719 into 1.x will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##                1.x   #5719      +/-   ##
===========================================
+ Coverage     84.18%   84.2%   +0.02%     
- Complexity     2886    2887       +1     
===========================================
  Files           290     290              
  Lines         18256   18256              
  Branches       2495    2495              
===========================================
+ Hits          15368   15373       +5     
  Misses         2000    2000              
+ Partials        888     883       -5
Impacted Files Coverage Δ Complexity Δ
src/main/java/rx/Observable.java 99.64% <ø> (ø) 449 <0> (ø) ⬇️
...rx/internal/util/atomic/MpscLinkedAtomicQueue.java 74.19% <0%> (-12.91%) 7% <0%> (-1%)
...ternal/operators/OperatorOnBackpressureLatest.java 78.84% <0%> (-3.85%) 3% <0%> (ø)
...x/internal/operators/DeferredScalarSubscriber.java 98.27% <0%> (-1.73%) 24% <0%> (-1%)
...in/java/rx/internal/operators/OperatorGroupBy.java 90.81% <0%> (-0.71%) 5% <0%> (ø)
...n/java/rx/internal/operators/CachedObservable.java 81.67% <0%> (+0.52%) 6% <0%> (ø) ⬇️
...main/java/rx/internal/operators/OperatorMerge.java 86.23% <0%> (+0.9%) 7% <0%> (ø) ⬇️
...ternal/operators/OperatorOnBackpressureBuffer.java 95% <0%> (+1.25%) 8% <0%> (ø) ⬇️
...n/java/rx/subscriptions/CompositeSubscription.java 76.62% <0%> (+1.29%) 25% <0%> (ø) ⬇️
...n/java/rx/subjects/SubjectSubscriptionManager.java 82.14% <0%> (+1.42%) 16% <0%> (+1%) ⬆️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b2394c...24bb5ca. Read the comment docs.

@@ -10535,6 +10535,8 @@ public final Subscription subscribe(Subscriber<? super T> subscriber) {
/**
* Returns an Observable that emits those items emitted by source Observable before a specified time runs
* out.
* If time runs out before Observable is finished, termination events would be called on provided {@link Scheduler},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A <p> would cleanly separate the first sentence and the rest of the Javadoc. Also the wording could use some refinement:

<p>
If time runs out before the {@code Observable} completes normally, the {@code onComplete} 
event will be signaled on the default {@code computation} {@link Scheduler}.

@@ -10559,6 +10561,8 @@ public final Subscription subscribe(Subscriber<? super T> subscriber) {
/**
* Returns an Observable that emits those items emitted by source Observable before a specified time (on a
* specified Scheduler) runs out.
* If time runs out before Observable is finished, termination events would be called on provided {@link Scheduler},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<p>
If time runs out before the {@code Observable} completes normally, the {@code onComplete} 
event will be signaled on the provided {@link Scheduler}.

@akarnokd akarnokd changed the title Add a sentence to documentation of take() operator 1.x: Add a sentence to documentation of take() operator Nov 11, 2017
Copy link
Collaborator

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreeing with David

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants