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

feat(ec2): add versionDescription property for LaunchTemplate #30837

Merged
merged 8 commits into from
Jul 24, 2024

Conversation

yuppe-kyupeen
Copy link
Contributor

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


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team July 12, 2024 06:46
@github-actions github-actions bot added p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jul 12, 2024
@yuppe-kyupeen yuppe-kyupeen changed the title feat(ec2): add versionDescription property for launch template feat(ec2): add versionDescription property for LaunchTemplate Jul 12, 2024
Comment on lines 224 to 231
/**
* 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;
Copy link
Contributor

@go-to-k go-to-k Jul 12, 2024

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.!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

Comment on lines 226 to 227
* The version descrioption must be maximum 255 characters long, including hyphens (-),
* underscores (_), spaces, and tabs.
Copy link
Contributor

@go-to-k go-to-k Jul 12, 2024

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.)

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-launchtemplate.html#cfn-ec2-launchtemplate-versiondescription

Comment on lines 225 to 226
* A description for the first version of the launch template.
* The version descrioption must be maximum 255 characters long, including hyphens (-),
Copy link
Contributor

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.

Suggested change
* 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 (-),

@yuppe-kyupeen
Copy link
Contributor Author

@go-to-k
Thank you for the review!
I have addressed the points you mentioned.I apologize for any oversights.
Please review the changes.Thank you!

Copy link
Contributor

@go-to-k go-to-k left a 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.

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 13, 2024
@yuppe-kyupeen
Copy link
Contributor Author

@go-to-k
thank you!

Copy link
Contributor

@gracelu0 gracelu0 left a 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!

packages/aws-cdk-lib/aws-ec2/lib/launch-template.ts Outdated Show resolved Hide resolved
@yuppe-kyupeen
Copy link
Contributor Author

Hi @gracelu0, thanks for your review and suggestion! 👍

Copy link
Contributor

mergify bot commented Jul 24, 2024

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-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 24, 2024
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 70c9be5
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 597228c into aws:main Jul 24, 2024
12 checks passed
Copy link
Contributor

mergify bot commented Jul 24, 2024

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).

Copy link

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.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@yuppe-kyupeen yuppe-kyupeen deleted the add-version-description-for-ec2 branch July 24, 2024 01:02
obraafo pushed a commit to obraafo/aws-cdk that referenced this pull request Jul 25, 2024
…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*
@aws-cdk-automation
Copy link
Collaborator

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.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants