fix(core): a number of resources are not taggable with `Tags.of()` by GavinZZ · Pull Request #28989 · aws/aws-cdk

GavinZZ · 2024-02-05T16:02:39Z

Issue # (if applicable)

Closes #28862

Reason for this change

CFN resources that has array format tags cannot use Tags.of() to add tags.

Description of changes

Enable modern style tags ITaggableV2

Description of how you validated changes

The generated looks correct and shouldn't cause breaking changes.

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

rix0rrr · 2024-02-05T17:20:37Z

What we usually do is phrase the PR title as user-visible changes (because the PR titles make it into the CHANGELOG). Since this fixes a bug, I would do something like:

fix(core): a number of resources are not taggable with `Tags.of()`

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

GavinZZ · 2024-02-05T18:50:39Z

Exemption Request
No existing test suite for spec2cdk tool.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

GavinZZ · 2024-02-08T02:46:30Z

@mergify update

mergify · 2024-02-08T02:46:35Z

update

✅ Branch has been successfully updated

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts

gossandr · 2024-02-15T06:14:53Z

Any update on this? I am looking forward to a new release ;-)

GavinZZ · 2024-02-22T06:55:50Z

@gossandr Will try to get this merged this week.

gossandr · 2024-02-22T07:44:07Z

@GavinZZ if you need any assistance, i might be able to help

mergify · 2024-02-23T10:22:49Z

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 · 2024-02-23T10:49:55Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 5cce6f5
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2024-02-23T10:50:13Z

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

github-actions bot added the bug This issue is a bug. label Feb 5, 2024

aws-cdk-automation requested a review from a team February 5, 2024 16:02

github-actions bot added effort/medium Medium work item – several days of effort p1 labels Feb 5, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Feb 5, 2024

aws-cdk-automation previously requested changes Feb 5, 2024

View reviewed changes

GavinZZ changed the title ~~feat(spec2cdk): enable modern tagging style with ITaggableV2~~ chore(spec2cdk): enable modern tagging style with ITaggableV2 Feb 5, 2024

rix0rrr reviewed Feb 5, 2024

View reviewed changes

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts Show resolved Hide resolved

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts Show resolved Hide resolved

GavinZZ changed the title ~~chore(spec2cdk): enable modern tagging style with ITaggableV2~~ fix(core): a number of resources are not taggable with Tags.of() Feb 5, 2024

aws-cdk-automation previously requested changes Feb 5, 2024

View reviewed changes

aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Feb 5, 2024

GavinZZ added the pr-linter/exempt-test The PR linter will not require test changes label Feb 5, 2024

GavinZZ marked this pull request as ready for review February 5, 2024 18:51

GavinZZ added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Feb 6, 2024

GavinZZ added 3 commits February 10, 2024 21:53

fix(spec2cdk): enable modern tagging style with ITaggableV2

dbe614c

Add a comment

c134260

Update code generation

5ff4515

GavinZZ force-pushed the yuanhaoz/tagv2 branch from ecd865a to 5ff4515 Compare February 10, 2024 15:08

GavinZZ requested a review from rix0rrr February 10, 2024 15:08

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 10, 2024

GavinZZ commented Feb 10, 2024

View reviewed changes

tools/@aws-cdk/spec2cdk/lib/cdk/resource-decider.ts Outdated Show resolved Hide resolved

GavinZZ had a problem deploying to test-pipeline February 22, 2024 08:13 — with GitHub Actions Failure

Remove unused boolean HAS_25610

e805aea

GavinZZ force-pushed the yuanhaoz/tagv2 branch from 773e3f4 to e805aea Compare February 22, 2024 08:13

GavinZZ added 2 commits February 22, 2024 16:39

Update render tags

1c374cd

Give property name in render tags

07529e6

rix0rrr approved these changes Feb 23, 2024

View reviewed changes

Merge branch 'main' into yuanhaoz/tagv2

5cce6f5

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 23, 2024

mergify bot merged commit 7a4c189 into main Feb 23, 2024

mergify bot deleted the yuanhaoz/tagv2 branch February 23, 2024 10:50

github-actions bot mentioned this pull request Mar 1, 2024

Monthly PR metrics report - Feb 2023 #29328

Closed

diranged mentioned this pull request Mar 7, 2024

(core): new tagging of existing AWS::EKS::AccessEntry resources is failing #29393

Closed

This was referenced Apr 17, 2024

spec2cdk: Breaking changes in Aws-cdk-lib-java from Tagging Change #29869

Closed

fix(spec2cdk): get tag gives null result in Java CDK #29870

Merged

Conversation

GavinZZ commented Feb 5, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

rix0rrr commented Feb 5, 2024

Uh oh!

Uh oh!

Uh oh!

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Uh oh!

GavinZZ commented Feb 5, 2024

Uh oh!

GavinZZ commented Feb 8, 2024

Uh oh!

mergify bot commented Feb 8, 2024

✅ Branch has been successfully updated

Uh oh!

Uh oh!

gossandr commented Feb 15, 2024

Uh oh!

GavinZZ commented Feb 22, 2024

Uh oh!

gossandr commented Feb 22, 2024

Uh oh!

mergify bot commented Feb 23, 2024

Uh oh!

aws-cdk-automation commented Feb 23, 2024

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Feb 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants