-
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
2.x: Expand the JavaDocs of the Scheduler API #5843
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #5843 +/- ##
============================================
- Coverage 96.38% 96.37% -0.01%
- Complexity 5812 5814 +2
============================================
Files 634 634
Lines 41760 41760
Branches 5796 5796
============================================
- Hits 40251 40248 -3
+ Misses 591 588 -3
- Partials 918 924 +6
Continue to review full report at Codecov.
|
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.
Looks good, but few picks
* units of work with or without delays or periodically. | ||
* You can get common instances of this class in {@link io.reactivex.schedulers.Schedulers}. | ||
* units of work provided in the form of {@link Runnable}s to be | ||
* executed immediately, after a specified time delay or periodically |
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.
nit: maybe replace immediately with something like "as soon as possible"?
* the {@link Worker#schedule(Runnable, long, TimeUnit)} for scheduling the {@code Runnable} task periodically. | ||
* The algorithm calculates the next absolute time when the task should run again and schedules this execution | ||
* based on the relative time between it and {@link Worker#now(TimeUnit)}. However, drifts or changes in the | ||
* system clock would affect this calculation either by scheduling subsequent runs too frequently or too far apart. |
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.
would -> could?
* Schedules a cancelable action to be executed periodically. This default implementation schedules | ||
* recursively and waits for actions to complete (instead of potentially executing long-running actions | ||
* concurrently). Each scheduler that can do periodic scheduling in a better way should override this. | ||
* Schedules a cancelable action to be executed periodically. |
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.
Maybe remove "cancelable" or add it to other "scheduleXXX" methods?
Other "scheduleXXX" methods also allow cancellation, but their javadoc only mentions it in the description of returned Disposable
This PR adds more details to the
Scheduler
andWorker
API, rewords some older sentences and fixes a few mistakes in others.In addition, the wording of the
SchedulerRunnableIntrospection
felt a bit clumsy and has been updated as well.