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
Adds normalizeHttpEvent Middleware #101
Conversation
Codecov Report
@@ Coverage Diff @@
## master #101 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 14 15 +1
Lines 261 267 +6
Branches 48 51 +3
=====================================
+ Hits 261 267 +6
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 stuff. Can you update tests as well?
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.
Fantastic.
Only 2 things before this can be ready for the merge:
- I would rename
withDefaultHttpEvent
tonormalizeHttpEvent
(see Api Gateway proxy event normalizer middleware #35) - We need middleware docs for
normalizeHttpEvent
(see https://github.com/middyjs/middy/blob/master/docs/middlewares.md) and update the list of middlewares accordingly in the README.md.hb
Hey @i-am-kenny, are you ok with the above comment? Do you need any kind of help/support? |
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 added docs and renamed the middleware. Getting this one merged and released soon
Merged, thanks for your precious contribution @i-am-kenny 😉 |
This should allow the CORS header to be applied even if the request fails. I can easily add the other headers/tests, but I wasn't sure about some reasonable defaults.
Closes #35