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

Issue #563 - completeL should handle F[_] errors, mapEval should not #567

Merged
merged 1 commit into from Jan 25, 2018

Conversation

Projects
None yet
1 participant
@alexandru
Member

alexandru commented Jan 25, 2018

Related to #563 — fixed completeL with a reasonable penalty for doing that error handling, due to mutating a plain var.

In case of this loop, it doesn't matter due to the inner workings of the loop being suspended and completely encapsulated. However this isn't an optimization that we can afford to do for operations returning Iterant.

Therefore, except for operations meant specifically for error handling, like onErrorHandlingWith, the errors raised in the F[_] context should get handled by folding operations (ending with the L suffix), one of which being completeL.

@alexandru alexandru self-assigned this Jan 25, 2018

@codecov

This comment has been minimized.

codecov bot commented Jan 25, 2018

Codecov Report

Merging #567 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #567      +/-   ##
==========================================
+ Coverage   90.57%   90.61%   +0.03%     
==========================================
  Files         369      369              
  Lines        9782     9781       -1     
  Branches     1841     1857      +16     
==========================================
+ Hits         8860     8863       +3     
+ Misses        922      918       -4
@alexandru

This comment has been minimized.

Member

alexandru commented Jan 25, 2018

@oleg-py Not waiting any longer on this PR, merging it directly to unblock your testing.

Implementation seems fine to me - the internal variable is completely encapsulated.

One note - mapEval no longer handles F[_] errors by itself, even if it still catches exceptions thrown directly from its function parameter (maybe we shouldn't do that either, since Task or IO will catch that exception anyway). The convention going forward is that the fold operations (like completeL) are in charge of handling F[_] errors. Or the onError* ops.

@alexandru alexandru merged commit 0728928 into monix:master Jan 25, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@alexandru alexandru added this to the 3.0.0 milestone Feb 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment