-
-
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 more doctests to Kleisli #2722
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2722 +/- ##
=======================================
Coverage 95.13% 95.13%
=======================================
Files 365 365
Lines 6798 6798
Branches 296 313 +17
=======================================
Hits 6467 6467
Misses 331 331
Continue to review full report at Codecov.
|
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.
Thanks, @nasadorian! Good stuff.
I left one minor comment. Also the git history is a little messy and seems to have some commits with duplicate titles. It may be a bit cleaner to squash some of those, but it's no big deal either way.
def pure[F[_], A, B](x: B)(implicit F: Applicative[F]): Kleisli[F, A, B] = | ||
Kleisli(_ => F.pure(x)) | ||
|
||
/** | ||
* Similar to [[pure]] except the input type is the same as the `A` in output type `F[A]`. | ||
*/ |
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.
This comment is true, but I think that it could be more illustrative. In my mind, ask
is to Kleisli
as identity
is to Function1
. I think that it would be helpful to point out that in the resulting Kleisli
, the output A
is the same as the input A
(the actual value not just the type). A code example might help get this point across.
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.
Just updated and added an example to ask
Thanks for the review @ceedubs - yes I think I'll squash the commits into one. |
512f15f
to
b35df7e
Compare
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.
LGTM, thanks very much!
Okay @kailuowang I got the build to pass, looks like my commits overrode your approval 😅 |
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.
🎉 thanks, @nasadorian!
Re: Issue #2479 -- Appreciate any feedback to improve or further clarify examples.
Adds comments and some doctests for Kleisli: