-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add a Binested data type #2259
Add a Binested data type #2259
Conversation
Looks useful to me 👍 |
I like it too, thanks very much! :) |
Codecov Report
@@ Coverage Diff @@
## master #2259 +/- ##
==========================================
+ Coverage 94.96% 94.98% +0.01%
==========================================
Files 334 336 +2
Lines 5805 5822 +17
Branches 218 211 -7
==========================================
+ Hits 5513 5530 +17
Misses 292 292
Continue to review full report at Codecov.
|
Added I think this is good to merge if all tests pass. |
} | ||
|
||
trait BinestedInstances0 { | ||
implicit def catsDataBitraverseForBinested[F[_, _], G[_], H[_]]( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitraverse
should take higher priority than Bifoldable
and Bifunctor
https://typelevel.org/cats/guidelines.html#a-idimplicit-priority-hrefimplicit-prioritya-implicit-instance-priority
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thanks
@@ -58,5 +58,6 @@ trait AllSyntaxBinCompat0 | |||
extends UnorderedTraverseSyntax | |||
with ApplicativeErrorExtension | |||
with TrySyntax | |||
with BinestedSyntax |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool. I'll rebase on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that sticking it on AllSyntaxBinCompat1
is fine. Will change to that...
fingers crossed for the MiMa gods |
oops, sorry, I squashed merged the other PR. Would you reset this branch to master and cherry pick your commit |
Yeah no worries. |
Rebased and resolved conflicts |
Travis failed 2 builds due to memory limits. Restarted the build... |
Resolves #2263.