feat(sns): add signature version prop#29543
Conversation
lpizzinidev
left a comment
There was a problem hiding this comment.
Looks good overall 👍
Left some suggestions for improvements
| public readonly topicName: string; | ||
| public readonly contentBasedDeduplication: boolean; | ||
| public readonly fifo: boolean; | ||
| public readonly signatureVersion?: string; |
There was a problem hiding this comment.
Do we need the public variable?
There was a problem hiding this comment.
What's the standard policy here? Prefer not to add it? This means take it out of topic-base.ts as well right ?
There was a problem hiding this comment.
I got rid of the public variable.
lpizzinidev
left a comment
There was a problem hiding this comment.
Thanks for the changes 👍
Left a comment for a minor adjustment, but overall looks good to me
Co-authored-by: Luca Pizzini <lpizzini7@gmail.com>
GavinZZ
left a comment
There was a problem hiding this comment.
Awesome, thanks for adding support for this property.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
@Mergifyio refresh |
✅ Pull request refreshed |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Closes #29539.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license