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

Task conversions (Cats-Effect 1.0.0-RC3 update, part 2) #686

Merged
merged 34 commits into from Sep 2, 2018

Conversation

Projects
None yet
3 participants
@alexandru
Member

alexandru commented Jul 11, 2018

This is a continuation of PR #685 by @oleg-py ... I forked that PR and continued it.

Changes:

  1. Task.start now forks, with fork being deprecated, done by @oleg-py in #685
  2. changed conversions to/from Task across the board, including their signatures
  • introduced for this purpose TaskLike and TaskLift type classes; changed Observable to use them instead of Effect because usage of Effect was wrong
  1. made unsafeCreate and Task.Context private, because a major internals refactoring is planned and it's better to cleanup the API right now, those standing in the way
  2. introduced SafeApp and deprecated TaskApp

alexandru and others added some commits Jul 17, 2017

@oleg-py

This PR is great 👍 All I can really do is nitpick, so here you go :)

Show resolved Hide resolved monix-eval/shared/src/main/scala/monix/eval/SafeApp.scala Outdated
Show resolved Hide resolved monix-eval/shared/src/main/scala/monix/eval/SafeApp.scala Outdated
@implicitNotFound("""Cannot find implicit value for TaskLift[${F}].
Building this implicit value might depend on having an implicit
s.c.ExecutionContext in scope, a Scheduler or some equivalent type.""")
trait TaskLift[F[_]] {

This comment has been minimized.

@oleg-py

oleg-py Jul 31, 2018

Collaborator

Maybe name it LiftTask for consistency with LiftIO? 🤔

This comment has been minimized.

@alexandru

alexandru Sep 1, 2018

Member

Meh, I like having Task prefix on the Task-related classes 🙂

Show resolved Hide resolved monix-eval/shared/src/main/scala/monix/eval/TaskLike.scala Outdated
Show resolved Hide resolved monix-eval/shared/src/main/scala/monix/eval/TaskLike.scala Outdated
Show resolved Hide resolved monix-eval/shared/src/main/scala/monix/eval/TaskLike.scala Outdated

@oleg-py oleg-py referenced this pull request Jul 31, 2018

Closed

WIP: Make Task#start fork; deprecate Task#fork #685

0 of 6 tasks complete

Avasil and others added some commits Aug 21, 2018

@alexandru alexandru changed the title from Task conversions (Cats-Effect 1.0.0-RC2 update, part 2) to Task conversions (Cats-Effect 1.0.0-RC3 update, part 2) Sep 1, 2018

alexandru added some commits Sep 1, 2018

@alexandru

This comment has been minimized.

Member

alexandru commented Sep 1, 2018

@oleg-py thanks a lot for the review, it was very useful and I addressed your comments.

alexandru added some commits Sep 1, 2018

@codecov

This comment has been minimized.

codecov bot commented Sep 2, 2018

Codecov Report

Merging #686 into master will decrease coverage by 5.9%.
The diff coverage is 95.32%.

@@            Coverage Diff             @@
##           master     #686      +/-   ##
==========================================
- Coverage   90.85%   84.94%   -5.91%     
==========================================
  Files         386      391       +5     
  Lines       10901    11733     +832     
  Branches     2059     2124      +65     
==========================================
+ Hits         9904     9967      +63     
- Misses        997     1766     +769

alexandru added some commits Sep 2, 2018

@alexandru alexandru merged commit 0f44a16 into master Sep 2, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment