-
Notifications
You must be signed in to change notification settings - Fork 402
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
fix(data-classes): docstring typos and clean up #937
Conversation
Changes: - Use correct wording: `built-in`, `its`, `sign in`, `all the` and `case-sensitive` - Modern use of `super()` - Use `startswith` instead of `resource[:1] == "/"`
Codecov Report
@@ Coverage Diff @@
## develop #937 +/- ##
===========================================
- Coverage 99.96% 99.96% -0.01%
===========================================
Files 119 119
Lines 5322 5320 -2
Branches 614 613 -1
===========================================
- Hits 5320 5318 -2
Partials 2 2
Continue to review full report at Codecov.
|
@heitorlessa - some small housekeeping fixes. |
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 question on whether you meant startswith
vs ==
, as I can't remember what the possible values can be.
aws_lambda_powertools/utilities/data_classes/api_gateway_authorizer_event.py
Outdated
Show resolved
Hide resolved
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 for the changes - one last one, please remove the empty logfile
, as it seems to have been added by accident.
@@ -28,11 +28,12 @@ def __init__( | |||
self.api_id = api_id | |||
self.stage = stage | |||
self.http_method = http_method | |||
self.resource = resource | |||
# Remove matching "/" from `resource`. |
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.
Thank you!
Updated description on |
Issue #, if available:
It's been a while since i scanned through the data classes for typos and improvements.
Description of changes:
Changes:
built-in
,its
,sign in
,all the
andcase-sensitive
super()
resource.lstrip("/")
instead ofresource[:1] == "/"
for readability to remove leading "/" that isn't allowed when constructing an authorizer response objectChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.