Skip to content
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

Merged
merged 9 commits into from
Dec 30, 2021
Merged

docs(all): clarifications & fixes #370

merged 9 commits into from
Dec 30, 2021

Conversation

dreamorosi
Copy link
Contributor

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 & run npm run docs-runLocalDocker.

Highlights:

#360

Before:
image

After: The inline code is now bolder & improves contrast
image

#359

Added note + tip regarding Middy under the first middleware code snippet of each utility:
image

#361

Moved "Patching AWS SDK clients" subsection under "Getting Started" section:
image

#362

Uniformed code example under "Capturing Lambda context info" subsection for Logger to use tabs like other utilities:

image

#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 log null as value of the keys related to the context.

image

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.

@dreamorosi dreamorosi added documentation Improvements or additions to documentation all labels Dec 29, 2021
@dreamorosi dreamorosi added this to the beta-release milestone Dec 29, 2021
@dreamorosi dreamorosi self-assigned this Dec 29, 2021
@dreamorosi dreamorosi requested a review from flochaz December 29, 2021 15:11
@@ -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.
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

@saragerion saragerion left a comment

Choose a reason for hiding this comment

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

Amazing!

@dreamorosi dreamorosi merged commit df0d9d3 into main Dec 30, 2021
@dreamorosi dreamorosi deleted the docs/all_clarifications branch December 30, 2021 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
3 participants