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

ReasonML edition. #247

Merged
merged 30 commits into from
Sep 9, 2020
Merged

ReasonML edition. #247

merged 30 commits into from
Sep 9, 2020

Conversation

fhammerschmidt
Copy link
Contributor

I converted the OCaml edition thanks to the Reason parser which can translate OCaml into Reason source code (and vice versa). Not everything did compile or typecheck, so there was still some manual work necessary.

I figured if I wanted to learn category theory, I'd rather do this in my everyday language so here's the ReasonML edition of CTFP!

@hmemcpy
Copy link
Owner

hmemcpy commented Mar 29, 2020

Oh wow! This is great, thank you :)
I don't speak ReasonML, so please feel free to tag people you'd think could give a hand in reviewing this! I'd be happy to merge it once you're satisfied with it!

@fhammerschmidt
Copy link
Contributor Author

Of course.

@yawaramin @mlms13 @andywhite37 @Risto-Stevcev
or anybody who feels like it, please review if you want.

src/Makefile Outdated Show resolved Hide resolved
@andywhite37
Copy link

@fhammerschmidt - this is really cool! I've been meaning to check out this book, so this is a good excuse. I don't know if I'll have time to do a thorough review, but the stuff I saw at a quick glance looked good.

@@ -0,0 +1,6 @@
module type Polymorphic_Function_F = {
Copy link

@JasoonS JasoonS Mar 30, 2020

Choose a reason for hiding this comment

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

I don't think these module types make it easy to understand if you are not very experienced with reason. I see in Yawar's PR to the ocaml version he also simplified it.

I guess it isn't correct to say let f: a => b; alone, but for pedagogical reasons I feel it is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update accordingly when his PR gets merged.

@fhammerschmidt
Copy link
Contributor Author

Can we please merge this?

It's probably not perfect for now, because I am still waiting for #242 to adapt, but I think it is better to have the bulk merged now.

@drupol
Copy link
Collaborator

drupol commented Sep 9, 2020

We need to also update the Github actions workflow to include this new edition.

@hmemcpy
Copy link
Owner

hmemcpy commented Sep 9, 2020

Yeah, very sorry for neglecting! :(
Would be happy if you rebased on top of the latest changes to make sure all build workflows are included.
Will be happy to merge then.

@fhammerschmidt
Copy link
Contributor Author

Rebased and added Reason to workflows: a537ebd

@hmemcpy
Copy link
Owner

hmemcpy commented Sep 9, 2020

Amazing, let's do this! 🎉

@hmemcpy hmemcpy merged commit 8336698 into hmemcpy:master Sep 9, 2020
@hmemcpy
Copy link
Owner

hmemcpy commented Sep 9, 2020

Thanks all very much for your contributions!!

@fhammerschmidt fhammerschmidt deleted the reason branch September 9, 2020 10:04
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.

5 participants