chore: make all L2 Constructs property injectable during release #34328
Conversation
github-actions
bot
added
the
beginning-contributor
pcheungamz
changed the title
pcheungamz
changed the title
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
aws-cdk-automation
added
the
pr/needs-community-review
| if (parts.length < 2) { | ||
| throw new Error(`Could not build PROPERTY_INJECTION_ID for ${filePath} ${className}`); | ||
| } | ||
| return `'aws-cdk-lib.${parts[0]}.${className}'`; |
There was a problem hiding this comment.
Sorry, I am not familiar with alpha modules. Can you give me some examples and I can add them to the unit tests.
There was a problem hiding this comment.
Modules in packages/@aws-cdk/ are alpha modules (aka experimental modules). The constructs in these modules should also be property injectable.
There was a problem hiding this comment.
I updated the logic to handle /packages/@awsk-cdk/...
| /** | ||
| * This adds import of prop-injectable to the file. | ||
| */ | ||
| private importPropertyInjectable(sourceFile: SourceFile, filePath: string) { |
There was a problem hiding this comment.
Can we group this method and the below addImportAndMetadataStatement's import part together into a function and make the function generic to handle different import statement input?
There was a problem hiding this comment.
Make sense. Let me combine them.
There was a problem hiding this comment.
addImportAndMetadataStatementn and makeConstructsPropInjectable both use importCoreLibFile
aws-cdk-automation
removed
the
pr/needs-community-review
aws-cdk-automation
added
the
pr/needs-community-review
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. |
aws-cdk-automation
removed
the
pr/needs-community-review
Issue # (if applicable)
This is a followup to #33887.
DO NOT merge this PR until after 33887 is merged.
Reason for this change
In the original Property Injection PR, we enabled 122 Constructs for Property Injections, but we want to make this feature available for all L2 Constructs.
Description of changes
Add logic to Constructs ConstructsUpdater to if a Construct already have PROPERTY_INJECTION_ID property. If not, it will:
Describe any new or updated permissions being added
No permission change.
Description of how you validated changes
npm test./bin/update-construct-metadatalocally and manually verified a few random files.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license