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

Abstract away from cats-effect IO #214

Merged
merged 1 commit into from
Jul 9, 2018
Merged

Abstract away from cats-effect IO #214

merged 1 commit into from
Jul 9, 2018

Conversation

BenFradet
Copy link
Contributor

@BenFradet BenFradet commented Jul 8, 2018

fixes #213

@BenFradet BenFradet changed the title [WIP] Derive capture from cats-effect Sync instead of IO Abstract away from cats-effect IO Jul 8, 2018
@BenFradet
Copy link
Contributor Author

Could you please have a look @juanpedromoreno @fedefernandez ?

@BenFradet BenFradet merged commit 18038f8 into master Jul 9, 2018
@BenFradet BenFradet deleted the bf-sync branch July 9, 2018 10:52
@juanpedromoreno
Copy link
Member

👍

fromFuture(runMap[A, F](rb, mapResponse))

/** Taken from Http4s **/
private def fromFuture[A](future: Eval[Future[A]])(implicit ec: ExecutionContext): F[A] =
Copy link

@kubukoz kubukoz Jul 9, 2018

Choose a reason for hiding this comment

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

Note that there was an issue about having this in cats-effect. Although it'd take a => Future[A], but that's a minor detail. Note that the suggested implementation would short-circuit in case of completed futures (wouldn't call async if the future was Future.successful or Future.failed): typelevel/cats-effect#199 (comment)

Copy link
Contributor Author

@BenFradet BenFradet Jul 9, 2018

Choose a reason for hiding this comment

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

The problem is that the reference implementation uses TrampolineEC and I didn't feel like copying that over too.

just understood what you meant, I'll see what we can change

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.

[cats-effect] Capture can be derived for anything that has a Sync instance
4 participants