-
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
feat(logger): JSDOCS support #491
Conversation
@@ -1,6 +1,13 @@ | |||
import type { Logger } from '../Logger'; | |||
import middy from '@middy/core'; | |||
|
|||
/** | |||
* Middy middleware that adds the current Lambda invocation's context | |||
* inside all log items. |
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.
an example can be useful here , and can will be more in line with what we have in tracer for instance: https://github.com/awslabs/aws-lambda-powertools-typescript/blob/main/packages/tracing/src/middleware/middy.ts#L15
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.
I agree, well spotted!
Co-authored-by: Florian Chazal <[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.
Left some minor comments/suggestion and looking forward to merge this!
packages/logger/src/Logger.ts
Outdated
@@ -18,6 +18,30 @@ import type { | |||
PowertoolLogData, | |||
} from './types'; | |||
|
|||
/** | |||
* | |||
* # Logger class |
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.
* information in all log items. | ||
* The decorator can be used only when attached to a Lambda function handler which | ||
* is written as method of a class, and should be declared just before the handler declaration. | ||
* |
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.
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
Co-authored-by: Andrea Amorosi <[email protected]>
…s-typescript into feat/jsdocs
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 job!
Description of your changes
Added support for JSDOCS: https://jsdoc.app
How to verify this change
N/A.
Related issues, RFCs
#354
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.