-
Notifications
You must be signed in to change notification settings - Fork 146
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(all): clarifications & fixes #370
Conversation
@@ -72,6 +72,12 @@ You can quickly start by importing the `Tracer` class, initialize it outside the | |||
|
|||
=== "Middleware" | |||
|
|||
!!! note | |||
Middy comes bundled with Tracer, so you can just import it when using the middleware. |
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.
Not in the scope of this PR: I don't think we should bundle middy. We only use it for its types, and now we are adding a prod dependency to developers' lambda functions they might actually not even use it.
I think we should require middy only as dev dependency.
cc: @dreamorosi @flochaz @ijemmy
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.
Agreed, I've opened an issue to track this at a later stage #373
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.
Amazing!
Description of your changes
This PR aims at addressing some of the early feedback for the documentation:
How to verify this change
Checkout
docs/all_clarifications
branch & runnpm run docs-runLocalDocker
.Highlights:
#360
Before:
After: The inline code is now bolder & improves contrast
#359
Added note + tip regarding Middy under the first middleware code snippet of each utility:
#361
Moved "Patching AWS SDK clients" subsection under "Getting Started" section:
#362
Uniformed code example under "Capturing Lambda context info" subsection for Logger to use tabs like other utilities:
#363
Added a new section "Testing your code" under Logger to clarify that if Customers want to have the context info in their logs they need to pass a dummy context, added also a mention to the
@aws-lambda-powertools/commons
package that Customers can use in case they don't want to create their own dummy context.For context: in the Python version of Powertools if no
context
object is passed then Logger will raise an exception. In the Typescript version instead Customers using Typescript must always pass an object of type Context as enforced by the type definition. With that said, if they choose to pass an empty object with type casting (i.e.{} as Context
), or they are using Javascript without strong types, Logger still won't throw an error but simply lognull
as value of the keys related to thecontext
.Related issues, RFCs
#358
#359
#360
#361
#362
#363
PR status
Is this ready for review?: YES
Is it a breaking change?: NO
Checklist
Breaking change checklist
N/A
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.