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

IOSSchedulers for RoboVM #1332

Merged
merged 2 commits into from
Jun 25, 2014
Merged

IOSSchedulers for RoboVM #1332

merged 2 commits into from
Jun 25, 2014

Conversation

ashleyj
Copy link
Contributor

@ashleyj ashleyj commented Jun 6, 2014

Added support for IOSchedulers for RoboVM

I haven't added unit tests as

  1. they can't be automated
  2. they require the simulator
    It sucks, I know and I'm working on a way to automate this.

Apologies for the double PR -- had to rebranch

@cloudbees-pull-request-builder

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

@Override
public Subscription schedule(final Action0 action, long delayTime, TimeUnit unit) {

ScheduledExecutorService executor = Executors.newScheduledThreadPool(1);
Copy link
Member

Choose a reason for hiding this comment

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

Starting a scheduled thread pool for each direct-running task is wasteful, consider implementing the schedule(Action0) directly and not by forwarding it to the timed overload.

Copy link
Member

Choose a reason for hiding this comment

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

Also, you may want to check if isUnsubscribed() at this point eagerly to prevent scheduling on a terminated worker.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, GenericScheduledExecutorService is package private but you need the same structure and single static instance.

Added single static instance of ExecutorService for delayed posting
Introduced ScheduledIOSAction to enable CompositeSubscription
@cloudbees-pull-request-builder

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

@cloudbees-pull-request-builder

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

@cloudbees-pull-request-builder

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

@benjchristensen
Copy link
Member

Thanks ... I'll get to this in the near future.

benjchristensen added a commit that referenced this pull request Jun 25, 2014
@benjchristensen benjchristensen merged commit a0e5cf1 into ReactiveX:master Jun 25, 2014
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.

4 participants