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

Fix #815: remove dependency on cats-effect from monix-execution #822

Merged
merged 4 commits into from Feb 10, 2019

Conversation

Projects
None yet
2 participants
@oleg-py
Copy link
Collaborator

commented Feb 1, 2019

Basically all the usages of cats-effect in execution are removed as discussed in #815.

For CancelableFuture instances, I've used a little known fact that package objects of parent packages are part of an implicit scope for type. That means if monix-execution does not define one, it can be defined in a separate project (monix-catnap defined one for monix.execution). Instances are immediately available without any extra imports, if you have catnap dependency.

This is an obscure hack, but it gives the best UX as in no extra imports are required. OTOH, it can be used to provide extension methods too. I've refrained from pushing it too hard for now.

I decided also to provide factories for Scheduler - cats-effect datatypes (SchedulerEffect, suggestion for better name are welcome) without syntax extensions - syntax doesn't really buy you much.

In #815, I forgot to mention Cancelable#toCancelToken. It has an optimization for dummy cancelables. We use it three times where it seems like it would not hit that optimization - scheduler calls and CancelableFuture usage.

@oleg-py oleg-py requested a review from alexandru Feb 1, 2019


import scala.concurrent.ExecutionContext

package object execution {

This comment has been minimized.

Copy link
@oleg-py

oleg-py Feb 1, 2019

Author Collaborator

This is the hacky bit. It might seem innocent, but note that it's actually in the catnap subproject.

This comment has been minimized.

Copy link
@alexandru

alexandru Feb 5, 2019

Member

This is an awesome hack if it allows us to keep the instances comfortable while removing the Cats-Effect dependency from monix-execution.

@oleg-py

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 2, 2019

Build is failing due to MiMa, I want to get opinion on the method used before I add exclusions

@alexandru
Copy link
Member

left a comment

@oleg-py this looks pretty good, love the package object hack.

I did not do a careful review of everything, but seems to be fine. Please wrap it up.

oleg-py added some commits Feb 5, 2019

@alexandru alexandru merged commit f2fbb5d into monix:master Feb 10, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.