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(cfnspec): cloudformation spec v18.4.0 #10493

Merged
merged 12 commits into from
Oct 1, 2020
Merged

Conversation

aws-cdk-automation
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation commented Sep 23, 2020

No description provided.

@NetaNir NetaNir requested review from NetaNir and removed request for NetaNir September 28, 2020 06:02
@NetaNir
Copy link
Contributor

NetaNir commented Sep 28, 2020

Need to add ApiEndpoint as an attribute to @aws-cdk/aws-awpigatwayv2 L2.

Comment on lines +171 to +172
* AWS::ECS::Service.NetworkConfiguration AwsvpcConfiguration (__deleted__)
* AWS::ECS::Service.NetworkConfiguration AwsVpcConfiguration (__added__)
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@NetaNir NetaNir Sep 29, 2020

Choose a reason for hiding this comment

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

As you mentioned in the issue, since the ECS resource is deployed in the CloudFormation Registry, its property types are case insensitive. Changing this now might result in resource replacement for existing users, we have seen something similar in #10201 (comment). Given this works now, and changing it might result in a breaking change, wouldn't be better to keep the previous casing?

Copy link
Contributor

@PatMyron PatMyron Sep 30, 2020

Choose a reason for hiding this comment

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

Given this works now, and changing it might result in a breaking change, wouldn't be better to keep the previous casing?

Seems better if ECS switches back to the original casing. Part of why I called out that context. The casing shouldn't have been changed in the first place

Copy link
Contributor

@shivlaks shivlaks Sep 30, 2020

Choose a reason for hiding this comment

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

@PatMyron @NetaNir makes sense - so a patch to change AwsVpcConfiguration back to AwsvpcConfiguration

@mergify
Copy link
Contributor

mergify bot commented Oct 1, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator Author

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: 8127319
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Oct 1, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit fa50369 into master Oct 1, 2020
@mergify mergify bot deleted the bump-cfnspec/v18.4.0 branch October 1, 2020 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants