Skip to content

chore(init-templates): fix import of init templates#21493

Merged
mergify[bot] merged 5 commits intoaws:mainfrom
lune-sta:fix_ts_init-templates
Aug 8, 2022
Merged

chore(init-templates): fix import of init templates#21493
mergify[bot] merged 5 commits intoaws:mainfrom
lune-sta:fix_ts_init-templates

Conversation

@lune-sta
Copy link
Copy Markdown
Contributor

@lune-sta lune-sta commented Aug 7, 2022 •
edited
Loading

TypeScript init-templates are not imported cdk.Duration.
Fixed to avoid confusion for new writing CDK.

All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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 August 7, 2022 23:10
@github-actions github-actions bot added the p2 label Aug 7, 2022
@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Aug 7, 2022

@mrgrain
Copy link
Copy Markdown
Contributor

mrgrain commented Aug 8, 2022

Hi @lune-sta thanks for your contribution. Please make sure you have working tests and added or updated tests where appropriate.

I support the motivation behind this change. Currently the created template would run, with your change it won't anymore since Duration is unused. I think the right way to solve this will be to move the core imports to import * as cdk from 'aws-cdk-lib'. We use that elsewhere in the templates already and recommend it for contributions to the libraries as well.

@lune-sta
Copy link
Copy Markdown
Contributor Author

lune-sta commented Aug 8, 2022

@mrgrain Thank you.
I modified it to use import * as cdk from 'aws-cdk-lib'.
The test is passing locally, let me check CI.

Test Suites: 59 passed, 59 total
Tests:       510 passed, 510 total
Snapshots:   0 total
Time:        18.386 s, estimated 22 s
Ran all test suites.
Tests successful.

mrgrain
mrgrain previously requested changes Aug 8, 2022
View reviewed changes
packages/aws-cdk/lib/init-templates/app/typescript/lib/%name%-stack.template.ts Outdated
@@ -1,3 +1,4 @@
// import * as cdk from 'aws-cdk-lib'
import { Stack, StackProps } from 'aws-cdk-lib';
Copy link
Copy Markdown
Contributor

@mrgrain mrgrain Aug 8, 2022

Choose a reason for hiding this comment

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

While we are at it, let's get rid of this line and change the usage further down to cdk.Stack etc.

Copy link
Copy Markdown
Contributor Author

@lune-sta lune-sta Aug 8, 2022

Choose a reason for hiding this comment

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

Understood, Thanks.

@mrgrain
Copy link
Copy Markdown
Contributor

mrgrain commented Aug 8, 2022

The test is passing locally, let me check CI.

Looks like this was blip in CI.

@mergify mergify bot dismissed mrgrain’s stale review August 8, 2022 11:39

Pull request has been modified.

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 8, 2022

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
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Aug 8, 2022

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: afb5123
  • 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 3e28f47 into aws:main Aug 8, 2022
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Aug 8, 2022

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

jmortlock pushed a commit to jmortlock/aws-cdk that referenced this pull request Aug 8, 2022
@lune-sta @jmortlock
chore(init-templates): fix import of init templates (aws#21493)
af5700b 
TypeScript init-templates are not imported `cdk.Duration`.
Fixed to avoid confusion for new writing CDK.

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@mrgrain mrgrain added the effort/small Small work item – less than a day of effort label Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mrgrain mrgrain mrgrain approved these changes

Assignees

No one assigned

Labels

effort/small Small work item – less than a day of effort p2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lune-sta @mrgrain @aws-cdk-automation