-
-
Notifications
You must be signed in to change notification settings - Fork 59
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
Generic request decoder and response encoder #353
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## 2.x.x #353 +/- ##
==========================================
- Coverage 83.73% 83.63% -0.10%
==========================================
Files 93 92 -1
Lines 5059 5047 -12
==========================================
- Hits 4236 4221 -15
- Misses 823 826 +3 ☔ View full report in Codecov by Sentry. |
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.
One small possible improvement - if it's helpful
associatedtype Decoder: HBRequestDecoder = NullDecoder | ||
associatedtype Encoder: HBResponseEncoder = NullEncoder |
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.
Does it make sense to make these primary associated types?
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'm not sure that is necessary. When are you going to need to specify the encoder in a parameter type
associatedtype Decoder: HBRequestDecoder = NullDecoder | ||
associatedtype Encoder: HBResponseEncoder = NullEncoder |
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 will need fixing in HBLambda
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.
yep, PR on its way
Instead of requiring we store a request decoder and response encoder in the core context. This PR makes them accessible from the root of
HBRequestContext
. This allows us to make them to be generic and not existential types.It does mean we have to get rid of
HBSetCodableMiddleware
, but I think making these generic is a bigger gain.