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(cloudformation-module): Improve Docs #1090

Merged
merged 13 commits into from
Mar 20, 2023
Merged

docs(cloudformation-module): Improve Docs #1090

merged 13 commits into from
Mar 20, 2023

Conversation

mriccia
Copy link
Contributor

@mriccia mriccia commented Mar 16, 2023

Issue #, if available: #1081

Description of changes:

Improving the Cloudformation custom resources documentation, including the new APIs introduced as part of this PR #1082

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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

msailes
msailes previously approved these changes Mar 16, 2023
Copy link
Contributor

@msailes msailes left a comment

Choose a reason for hiding this comment

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

Perfect

scottgerring
scottgerring previously approved these changes Mar 16, 2023
### Configuring Response Objects
In both of the scenarios, powertools-java will assign the `physicalResourceId` based on the following logic:
- if present, use the `physicalResourceId` provided in the `CloudFormationCustomResourceEvent`
- if it is not present, use the `LogStreamName` from the Lambda context
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could be more explicit here. The physicalResourceId will only be missing for CREATE - "For CREATE operations, the logStreamName will be used. For UPDATE and DELETE, the physicalResourceId previously given by the custom resource to CloudFormation will be provided, back to the handler.", or something

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, it would be easier to understand this way, I will update the verbiage

mriccia and others added 7 commits March 20, 2023 11:15
improvements from PR

Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
improvements from PR

Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
improvements from PR

Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
improvements from PR

Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
PR Feedback

Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
Copy link
Contributor

@msailes msailes left a comment

Choose a reason for hiding this comment

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

LGTM

@msailes msailes merged commit f8731bf into aws-powertools:master Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants