Skip to content

make sure that EitherT MonadError syntax works the old way - #2029

Merged
LukaJCB merged 3 commits into
typelevel:masterfrom
kailuowang:fix-me-syntax-eitherT
Nov 14, 2017
Merged

make sure that EitherT MonadError syntax works the old way#2029
LukaJCB merged 3 commits into
typelevel:masterfrom
kailuowang:fix-me-syntax-eitherT

Conversation

@kailuowang

@kailuowang kailuowang commented Nov 14, 2017

Copy link
Copy Markdown
Contributor

flipped the priority sequence between the two MonadError instances for EitherT so that the syntax goes back to the behavior prior to 1.0-RC1. If user need the MonadError syntax with the MonadError instance that deals with the Error type on the inner F, they would need to create a local implicit instance that takes higher precedence, see the RegressionSuite for example
fixes #2022

@kailuowang kailuowang added this to the 1.0.0 milestone Nov 14, 2017

import ExtraRegressionSuite._

class ExtraRegressionSuite extends CatsSuite {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are we deleting this? 🤔

@kailuowang kailuowang Nov 14, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oops I did a rename but didn't include the renamed file in the git. just pushed. good catch.

@LukaJCB LukaJCB left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM except for that one comment I left below :)

LukaJCB
LukaJCB previously approved these changes Nov 14, 2017
johnynek
johnynek previously approved these changes Nov 14, 2017
@kailuowang
kailuowang dismissed stale reviews from johnynek and LukaJCB via 1a8e545 November 14, 2017 18:04
@kailuowang
kailuowang force-pushed the fix-me-syntax-eitherT branch from 67f5861 to 1a8e545 Compare November 14, 2017 18:04
@codecov-io

codecov-io commented Nov 14, 2017

Copy link
Copy Markdown

Codecov Report

Merging #2029 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2029   +/-   ##
=======================================
  Coverage   95.09%   95.09%           
=======================================
  Files         301      301           
  Lines        4953     4953           
  Branches      125      125           
=======================================
  Hits         4710     4710           
  Misses        243      243
Impacted Files Coverage Δ
core/src/main/scala/cats/data/EitherT.scala 97.58% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 323f2d5...1a8e545. Read the comment docs.

@kailuowang

kailuowang commented Nov 14, 2017

Copy link
Copy Markdown
Contributor Author

@LukaJCB, @johnynek I added an mima exception to get it pass the build, either of you have time to give a quick re-approval?

@LukaJCB
LukaJCB merged commit c5f76a2 into typelevel:master Nov 14, 2017
@stew stew removed the in progress label Nov 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't use MonadError syntax for EitherT

5 participants