-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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(ec2): add versionDescription
property for LaunchTemplate
#30837
feat(ec2): add versionDescription
property for LaunchTemplate
#30837
Conversation
versionDescription
property for LaunchTemplate
/** | ||
* A description for the first version of the launch template. | ||
* The version descrioption must be maximum 255 characters long, including hyphens (-), | ||
* underscores (_), spaces, and tabs. | ||
* | ||
* @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-launchtemplate.html#cfn-ec2-launchtemplate-versiondescription | ||
*/ | ||
readonly versionDescription?: string; |
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.
The CodeBuild CI seems to be failing. Optional properties must have @default
documentation. Could you add the label and description to the JSDoc?
aws-cdk-lib: error: [awslint:props-default-doc:aws-cdk-lib.aws_ec2.LaunchTemplateProps.versionDescription] Optional property must have @default documentation
aws-cdk-lib: Error: /codebuild/output/src1195990305/src/github.com/aws/aws-cdk/tools/@aws-cdk/cdk-build-tools/bin/cdk-awslint exited with error code 1
aws-cdk-lib: Build failed.!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
* The version descrioption must be maximum 255 characters long, including hyphens (-), | ||
* underscores (_), spaces, and tabs. |
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.
including hyphens (-),
* underscores (_), spaces, and tabs.
Is there documentation available to confirm this? (I could not find that in the CFn documentation.)
* A description for the first version of the launch template. | ||
* The version descrioption must be maximum 255 characters long, including hyphens (-), |
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.
A line break would make it clearer.
* A description for the first version of the launch template. | |
* The version descrioption must be maximum 255 characters long, including hyphens (-), | |
* A description for the first version of the launch template. | |
* | |
* The version descrioption must be maximum 255 characters long, including hyphens (-), |
@go-to-k |
Co-authored-by: Kenta Goto (k.goto) <24818752+go-to-k@users.noreply.github.com>
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.
Thanks for the changes. Approved.
@go-to-k |
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.
Hi @yuppe-kyupeen , thanks for your contribution. Just one small typo to fix, otherwise LGTM!
Co-authored-by: Grace Luo <54298030+gracelu0@users.noreply.github.com>
Hi @gracelu0, thanks for your review and suggestion! 👍 |
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). |
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). |
Comments on closed issues and PRs are hard for our team to see. |
…s#30837) ### Issue # (if applicable) ### Reason for this change The change introduces the `versionDescription` property to the `LaunchTemplate` ### Description of changes - Add the `versionDescription` property for `LaunchTemplateProps`, which was missing in the L2 construct. - Add validation for character limit ### Description of how you validated changes I Added a unit test for launch template and added the `versionDescription` property in the integration tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one. |
Issue # (if applicable)
Reason for this change
The change introduces the
versionDescription
property to theLaunchTemplate
Description of changes
versionDescription
property forLaunchTemplateProps
, which was missing in the L2 construct.Description of how you validated changes
I Added a unit test for launch template and added the
versionDescription
property in the integration tests.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license