-
Notifications
You must be signed in to change notification settings - Fork 421
docs(event_handler): add information about case-insensitive header lookup function #3183
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
docs(event_handler): add information about case-insensitive header lookup function #3183
Conversation
Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need. |
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.
Hi @pgrzesik! I left a comment with a suggestion to transform the text into a more active voice and tell the customer what they should do to achieve the result they want. Make sense?
Thank you for sending this PR. Improving our documentation from customer feedback is something we work hard on every day.
Co-authored-by: Leandro Damascena <[email protected]> Signed-off-by: Piotr Grzesik <[email protected]>
Thank you for the suggestion @leandrodamascena, I just accepted it 🙌 |
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.
Hi @pgrzesik! This PR seems simple because we just changed a line, but the discussions that brought us here were complex and you could find a simple and effective solution. This is priceless, we really appreciate this.
Approved!
Signed-off-by: Heitor Lessa <[email protected]>
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.
looks great, thank you so much @pgrzesik!
@leandrodamascena before we merge, we need to fix the docstring in the get_header_value
function to signal that by default we make a case-insensitive lookup*
Hey @heitorlessa - https://github.com/aws-powertools/powertools-lambda-python/pull/3183/files#diff-201518b4cf27a31cff68870554e0b4e2d6de2fa18456d2ea85d04d156991600cR192 |
Kudos, SonarCloud Quality Gate passed!
|
Codecov ReportAll modified lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #3183 +/- ##
========================================
Coverage 95.96% 95.96%
========================================
Files 195 195
Lines 8381 8381
Branches 1562 1562
========================================
Hits 8043 8043
Misses 276 276
Partials 62 62
☔ View full report in Codecov by Sentry. |
Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience! |
Issue number: #3142
Summary
Slightly improved documentation about case-insensitive lookup of specific header values for REST API events.
Changes
As above, slight changes to docs only
User experience
Improved documentation
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.