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.

Sign up for GitHub

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

Jump to bottom

Delete unnecessary restriction on biFlatMap #2837

Merged
merged 4 commits into from
May 10, 2019
Merged

Delete unnecessary restriction on biFlatMap #2837

merged 4 commits into from
May 10, 2019

Conversation

AdrianRaFo
Copy link
Contributor

This PR delete an unnecessary type restriction on the EitherT#biFlatMap implementation

@codecov-io
Copy link

codecov-io commented May 10, 2019 •
edited
Loading

Codecov Report

Merging #2837 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2837   +/-   ##
=======================================
  Coverage   94.21%   94.21%           
=======================================
  Files         368      368           
  Lines        6944     6944           
  Branches      301      301           
=======================================
  Hits         6542     6542           
  Misses        402      402
Impacted Files Coverage Δ
core/src/main/scala/cats/data/EitherT.scala 97.93% <ø> (ø) ⬆️

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 4894122...9dd82ec. Read the comment docs.

@kailuowang
Copy link
Contributor

sorry I didn't make it clear. I didn't mean to change all AA and BB to C and D, only those that do not have a AA :> A qualifier ones. I think it makes sense to differentiate between transforming a type to a new type and widening it.

@AdrianRaFo
Copy link
Contributor Author

AdrianRaFo commented May 10, 2019 •
edited
Loading

Oh ok, that makes sense

kailuowang
kailuowang approved these changes May 10, 2019
View reviewed changes
Copy link
Contributor

@kailuowang kailuowang left a comment

Choose a reason for hiding this comment

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

Thanks !!!

@AdrianRaFo
Copy link
Contributor Author

Could you merge it? I have no permissions to do that

@kailuowang
Copy link
Contributor

kailuowang commented May 10, 2019 •
edited
Loading

@AdrianRaFo we have a policy of requiring two maintainer sign offs before merging. Calling @LukaJCB ...

@kailuowang
Copy link
Contributor

thanks @djspiewak

@kailuowang kailuowang merged commit 912a14a into typelevel:master May 10, 2019
@AdrianRaFo AdrianRaFo deleted the arf-delete-eithert-type-restr branch May 10, 2019 23:22
@rossabaker rossabaker mentioned this pull request May 26, 2019
Merged
@diesalbla diesalbla mentioned this pull request May 26, 2019
Open
@kailuowang kailuowang added this to the 2.0.0-M2 milestone Jun 4, 2019
@avdv
Copy link

avdv commented Jun 5, 2019

❤️ Nice! I just noticed the same restriction and almost gave in to the reflex of creating a PR... ;-)

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

@djspiewak djspiewak djspiewak approved these changes

@kailuowang kailuowang kailuowang approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
2.0.0-M2
Development

Successfully merging this pull request may close these issues.
5 participants
@AdrianRaFo @codecov-io @kailuowang @avdv @djspiewak