-
Notifications
You must be signed in to change notification settings - Fork 448
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
#266: Alias constructors for applicative builders for Try, Eval, IO #278
Conversation
Codecov Report
@@ Coverage Diff @@
## master #278 +/- ##
=========================================
Coverage ? 42.49%
Complexity ? 318
=========================================
Files ? 148
Lines ? 3949
Branches ? 402
=========================================
Hits ? 1678
Misses ? 2150
Partials ? 121
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.
Great first PR! Can you please rename all the constructors to merge, instead of parallel, for the reasons we spoke about yesterday.
Once that's done, we're ready to merge!
Sure, I just thought it was just fitting for |
invoke(op3), | ||
invoke(op4)).ev() | ||
|
||
fun <A, B, C, D, E> merge( |
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.
Regarding codestyle we should follow the same conventions as in https://github.com/kategory/kategory/blob/master/kategory-core/src/main/kotlin/kategory/typeclasses/Applicative.kt. Alternatively we can change the ones in Applicative to match these one. I'll leave up to you.
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 don't see one clear way of formatting it inApplicative.kt
.
Thank you @anstaendig for the PR! I hope it helped you understand Kategory and FP a bit better :D Do ping us on Slack if you want another ticket, we can ramp up the difficulty :P |
Issue: #266