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

Make Task auto-cancelable #724

Merged
merged 4 commits into from Sep 21, 2018

Conversation

Projects
None yet
1 participant
@alexandru
Member

alexandru commented Sep 21, 2018

This is a second proposal, as a competitor to #721. In this one I'm not touching Task.Options.

Unfortunately I realized that the implementation in #721 has flaws due to Task's internals and I can't expose Resource without making the internal Context mutable.

PR changes:

  1. Task becomes auto-cancelable by default
  2. The autoCancelable helper is gone
  3. Task#memoize and memoizeOnSuccess are now uncancelable: it was a mistake to leave the possibility for any of the consumers to cancel the source
  4. Ditto for Fiber#join ... cancelling it no longer cancels the source, only Fiber#cancel does, this following a similar change in Cats-Effect
  5. Optimized executeWithOptions to be based on ContextSwitch and to no longer do any asynchrony

alexandru added some commits Sep 21, 2018

@codecov

This comment has been minimized.

codecov bot commented Sep 21, 2018

Codecov Report

Merging #724 into master will decrease coverage by 0.21%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##           master     #724      +/-   ##
==========================================
- Coverage   90.81%   90.59%   -0.22%     
==========================================
  Files         392      392              
  Lines       11013    11004       -9     
  Branches     2080     2056      -24     
==========================================
- Hits        10001     9969      -32     
- Misses       1012     1035      +23

@alexandru alexandru merged commit 88f6fa8 into monix:master Sep 21, 2018

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