-
Notifications
You must be signed in to change notification settings - Fork 199
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
Migrated CloudWatch Insights to new CoreAddons model #994
Migrated CloudWatch Insights to new CoreAddons model #994
Conversation
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.
@5herlocked Minor feedback on versionMap
, @shapirov103 double check from your end. This is for issue #971
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.
@5herlocked have you validated that 1/ addon creates correct service account and 2/ you can set the version explicitly, or auto as well ?
I have validated both |
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.
@5herlocked please address the minor comment, before submitting please make sure you run all commands specified https://github.com/aws-quickstart/cdk-eks-blueprints/blob/main/docs/internal/readme-internal.md#submitting-pull-requests.
npm i
make build
make lint
make run-test
cdk list
.addOns(new blueprints.CloudWatchInsights()) | ||
.buildAsync(app, 'cloudwatch'); | ||
|
||
const template = Template.fromStack(blueprint); |
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.
github code quality issue here. please run make lint
fc817b1
to
7f5d5b9
Compare
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.
LGTM, thank you!
Issue #, if available: #971, #948
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.