Skip to content

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Jan 4, 2022

Issue #, if available:

It's been a while since i scanned through the data classes for typos and improvements.

Description of changes:

Changes:

  • Use correct wording: built-in, its, sign in, all the and case-sensitive
  • Modern use of super()
  • Use resource.lstrip("/") instead of resource[:1] == "/" for readability to remove leading "/" that isn't allowed when constructing an authorizer response object

Checklist

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Changes:
- Use correct wording: `built-in`, `its`, `sign in`, `all the` and `case-sensitive`
- Modern use of `super()`
- Use `startswith` instead of `resource[:1] == "/"`
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 4, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2022

Codecov Report

Merging #937 (4203d4a) into develop (f2a4818) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             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              
Impacted Files Coverage Δ
aws_lambda_powertools/event_handler/api_gateway.py 100.00% <ø> (ø)
...s/utilities/data_classes/appsync_resolver_event.py 100.00% <ø> (ø)
.../utilities/data_classes/cognito_user_pool_event.py 100.00% <ø> (ø)
...s/utilities/data_classes/dynamo_db_stream_event.py 100.00% <ø> (ø)
...wertools/utilities/data_classes/s3_object_event.py 100.00% <ø> (ø)
...ities/data_classes/api_gateway_authorizer_event.py 100.00% <100.00%> (ø)
...bda_powertools/utilities/data_classes/sqs_event.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2a4818...4203d4a. Read the comment docs.

@michaelbrewer
Copy link
Contributor Author

@heitorlessa - some small housekeeping fixes.

Copy link
Contributor

@heitorlessa heitorlessa left a 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.

Copy link
Contributor

@heitorlessa heitorlessa left a 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`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@heitorlessa heitorlessa changed the title fix(data-classes): fix some small typos in doc strings and clean up fix(data-classes): docstring typos and clean up Jan 12, 2022
@heitorlessa
Copy link
Contributor

Updated description on .lstrip to clarify what this is for for anyone looking at this in the future.

@heitorlessa heitorlessa merged commit 24ab726 into aws-powertools:develop Jan 12, 2022
@heitorlessa heitorlessa deleted the docs-typo branch January 12, 2022 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants