Skip to content

feat(remodel): generate index.ts and .jsiirc.json files#23970

Merged
madeline-k merged 3 commits intofeat/repo-restructurefrom
madeline-k/gen-files-for-new-modules
Feb 2, 2023
Merged

feat(remodel): generate index.ts and .jsiirc.json files#23970
madeline-k merged 3 commits intofeat/repo-restructurefrom
madeline-k/gen-files-for-new-modules

Conversation

@madeline-k
Copy link
Copy Markdown
Contributor

@madeline-k madeline-k commented Feb 2, 2023 •
edited
Loading

This PR modifies the gen.ts script that will run in aws-cdk-lib after the remodel.

  1. I fixed a minor issue where the L1s were getting generated under aws-cdk-lib/lib/aws-service/service.generated.ts when it should be: aws-cdk-lib/aws-service/lib/service.generated.ts.
  2. Generate index.ts files if they are not there
    a. In main, the only automation to do this exists in create-missing-libraries.ts, so I've lifted some inspiration from there.
  3. Generate .jsiirc.json files if they are not there.
    a. In main, this is currently done by ubergen here, by applying a transform to the jsii targets in each package.json. This seems unnecesarily complicated to me.

Remaining todo:

  • Run aws-cdk-lib's gen script at the end of the cloudformation spec update.sh script. This will cause the update job to check-in .jsiirc.json and index.ts files for new modules.

    Question: Should I just modify this in place on our branch code link? Or make the remodel tool do it?

  • Mark create-missing-libraries.ts for deletion by the remodel tool

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

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Feb 2, 2023

@aws-cdk-automation aws-cdk-automation requested a review from a team February 2, 2023 16:03
@github-actions github-actions bot added the p2 label Feb 2, 2023
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 2, 2023
aws-cdk-automation
aws-cdk-automation requested changes Feb 2, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

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.

@MrArnoldPalmer
Copy link
Copy Markdown
Contributor

MrArnoldPalmer commented Feb 2, 2023

I fixed a minor issue where the L1s were getting generated under aws-cdk-lib/lib/aws-service/service.generated.ts when it should be: aws-cdk-lib/aws-service/lib/service.generated.ts.

So in our case I don't think we actually need the intermediary lib dir as that is not actually what the export paths are since they are defined in the package.json exports field. Also the top level lib directory serves to keep the large list of service directories out of the module root which makes it hard to work on imo. Otherwise LGTM

@madeline-k madeline-k merged commit 7813aa7 into feat/repo-restructure Feb 2, 2023
@madeline-k madeline-k deleted the madeline-k/gen-files-for-new-modules branch February 2, 2023 23:25
@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

aws-cdk-automation commented Feb 2, 2023

The pull request linter fails with the following errors:

❌ Features must contain a change to a README file.
❌ Features must contain a change to a test file.
❌ Features must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aws-cdk-automation aws-cdk-automation aws-cdk-automation requested changes

+1 more reviewer

@MrArnoldPalmer MrArnoldPalmer MrArnoldPalmer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

contribution/core This is a PR that came from AWS. p2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@madeline-k @MrArnoldPalmer @aws-cdk-automation