-
Notifications
You must be signed in to change notification settings - Fork 222
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
Introduce finch-generic module #716
Conversation
Codecov Report
@@ Coverage Diff @@
## master #716 +/- ##
==========================================
+ Coverage 79.78% 81.01% +1.22%
==========================================
Files 35 37 +2
Lines 653 653
Branches 18 21 +3
==========================================
+ Hits 521 529 +8
+ Misses 132 124 -8
Continue to review full report at Codecov.
|
8c23bb4
to
c2586c9
Compare
@vkostyukov @rpless any chance to get a feedback here? |
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 @ilya-murzinov! (Sorry it took me a while to review). This LGTM!
/** | ||
* Generically derive a very basic instance of [[Endpoint]] for a given type `A`. | ||
*/ | ||
def deriveFor[A]: GenericDerivation[A] = new GenericDerivation[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.
Since we're moving this anyway, how about we change that to deriveEndpoint[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.
Done
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.
Aside from renaming deriveFor
, 👍
Hey @ilya-murzinov! Any chance you'd be able to update this branch today/tomorrow? I was hoping to get the release out and wanted to include these changes there as well. |
Sure, will do tomorrow!
…On Wed, Mar 29, 2017, 20:34 Vladimir Kostyukov ***@***.***> wrote:
Hey @ilya-murzinov <https://github.com/ilya-murzinov>! Any chance you'd
be able to update this branch today/tomorrow? I was hoping to get the
release out and wanted to include these changes there as well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#716 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADiGKxE6q9vXxheyoAq6mS2uiLiRQTaOks5rqpYhgaJpZM4Ldr1x>
.
|
b5297e0
to
b657e37
Compare
/** | ||
* Generically derive a very basic instance of [[Endpoint]] for a given type `A`. | ||
*/ | ||
def deriveEndpointFor[A]: GenericDerivation[A] = new GenericDerivation[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.
Sorry, mind dropping For
? Just deriveEndpoint
is perfectly fine.
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 kinda liked this 'for', but since you insist...
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.
Yeah, I can see why you might like it and yet I think it doesn't really add much readability. Also, I learn from Circe that has deriveDecoder
and deriveEncoder
.
a45e9ae
to
333e9a1
Compare
333e9a1
to
f6093f9
Compare
Before:
After:
Resolves #528