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.

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

WIP: cats 0.7 #333

Closed
wants to merge 2 commits into from
Closed

WIP: cats 0.7 #333

wants to merge 2 commits into from

Conversation

KadekM
Copy link
Contributor

@KadekM KadekM commented Sep 1, 2016

I tried to see how it would look if we updated cats to 0.7...

I didn't spend much time on it but from quick peek I'm not exactly sure what the right way with last two remaining errors regarding cats.RecursiveTailRecM[M] (any tips?). I'll come back to it later and give it a try.

@tpolecat
Copy link
Member

tpolecat commented Sep 1, 2016

Ugh, yeah this is going to suck. We need to add RecursiveTailRecM as a constraint everywhere we have [M[_]: Monad: Catchable: Capture] which means a ton of new yax directives.

I'll experiment and see how bad this is going to be. There are 5491 instances but most are in generated code.

@KadekM
Copy link
Contributor Author

KadekM commented Sep 1, 2016

I kind of hoped there would be cleaner way than spread YAX all over the place :/

@tpolecat
Copy link
Member

tpolecat commented Sep 1, 2016

Yeah I need to drink some bourbon and think about our options.

I really appreciate your work on this. Many many thanks.

@KadekM
Copy link
Contributor Author

KadekM commented Sep 1, 2016

I pushed some yax that I had started beforehand...

but hopefully you'll find out some better way because this is... suboptimal :(

@guersam
Copy link
Contributor

guersam commented Sep 2, 2016

I've chosen plain Monad + Free#foldMapUnsafe instead of adding RecursiveTailRecM constraint everywhere for easier compatibility. Is it one of the hard requirement of our algebra to ensure 100% stack safety anyway?

@guersam
Copy link
Contributor

guersam commented Sep 2, 2016

@tpolecat
Copy link
Member

tpolecat commented Sep 3, 2016

Ok sorry, I haven't been keeping up closely. @guersam it looks like this work is subsumed by your work in progress on #323, is that correct?

@guersam
Copy link
Contributor

guersam commented Sep 3, 2016

Right, it includes cats 0.7.0 + fs2 0.9.0-RC2 support.

@KadekM
Copy link
Contributor Author

KadekM commented Sep 3, 2016

Great :) I'll close this then.

btw latest cats now is 0.7.2

@KadekM KadekM closed this Sep 3, 2016
@guersam
Copy link
Contributor

guersam commented Sep 3, 2016

Didn't know about that, thanks for pointing it out!

On Sat, Sep 3, 2016, 4:02 PM Marek Kadek notifications@github.com wrote:

Closed #333 #333.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#333 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA7JoGSOKtVijYzGwaKOaxFN7V452l4lks5qmRucgaJpZM4JyPfQ
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants