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

Add lift to FunctionK #1352

Merged
merged 7 commits into from
Sep 8, 2016
Merged

Conversation

andyscott
Copy link
Contributor

@andyscott andyscott commented Sep 3, 2016

Quick attempt at providing a lift method for FunctionK, by means of a macro.

In my local testing, this works for Free and other more complicated types if you've enabled some form of the SI-2712 fix.

Thoughts?

@andyscott
Copy link
Contributor Author

Everything passes except some style assertions, such as using null.

@non
Copy link
Contributor

non commented Sep 3, 2016

Thanks!

Would you mind writing some Scaladoc to explain what the macro does, how to use it, etc.? I think it will be relatively hard for casual users to look at this and understand how to use it.

@codecov-io
Copy link

codecov-io commented Sep 3, 2016

Current coverage is 91.72% (diff: 88.23%)

Merging #1352 into master will decrease coverage by 0.01%

@@             master      #1352   diff @@
==========================================
  Files           237        238     +1   
  Lines          3571       3588    +17   
  Methods        3505       3520    +15   
  Messages          0          0          
  Branches         65         67     +2   
==========================================
+ Hits           3276       3291    +15   
- Misses          295        297     +2   
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 0408170...e819893

@kailuowang
Copy link
Contributor

Thanks very much! I like the idea (yah! less boilerplate for Free) and the test and doc looks good to me, but I am not too qualified to reviewed the macro part, other than @non, maybe @fthomas ?

@andyscott andyscott changed the title Add lift to FunctionK (via a macro) Add lift to FunctionK Sep 3, 2016
@andyscott
Copy link
Contributor Author

Less boilerplate for Free was exactly what motivated me to write this! I am considering using free monads on a project where a number of people have worked minimally in Scala, so I've been crafting a demo project that minimizes as many "scary" concepts as possible.

@andyscott
Copy link
Contributor Author

Do I need to fix the code coverage issues for this PR? I'm not entirely sure how I'd be able to provide coverage for some of the "internal" methods to the macro.

@non
Copy link
Contributor

non commented Sep 6, 2016

@andyscott I think it's fine -- I'm not sure if those error message can be hit, but I'm glad you're providing an error message in case someone does hit them.

LGTM 👍

@johnynek
Copy link
Contributor

johnynek commented Sep 8, 2016

👍

@johnynek johnynek merged commit a0278dd into typelevel:master Sep 8, 2016
@ghost ghost mentioned this pull request Sep 14, 2016
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