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(remodel): call ubergen to create aws-cdk-lib #24263

Conversation

MrArnoldPalmer
Copy link
Contributor

Updates the remodel package to correctly generate the aws-cdk-lib package source code. Exposes a bunch of functions from within the ubergen package, which remodel now calls in order to copy all of the expected packages from the packages/@aws-cdk directory into packages/aws-cdk-lib/lib while rewriting import paths within the source code.

Additionally, ubergen returns a list of the bundled packages, which we use to build a map of the output directories to the correct cloudformation resource scopes. This is necessary because some scopes are repeated throughout different packages and are generated different depending on whether they are generated within a package that has other potentially related scopes, or not. This scope map is output to aws-cdk-lib/scripts/scope-map.json for use during code generation.

cfn2ts now reads this scope map, and will update it with newly generated directories/scopes as needed.

The verify-imports-resolve-same script currently fails. Investigating this separately as I believe the outputs are correct and it may just be a matter of adding additional entries to package.json exports field.

The remodel tool now calls some functions from ubergen in order to
create the aws-cdk-lib package in a way that is consistent with how it
is built today. Some differences from ubergen are required since it
assumes that codegen has already taken place, and that L1s need to be
extracted from experimental modules, which is not the case when working
on aws-cdk-lib directly.

Additionally makes a bunch of the functions in ubergen exported and
changes them to rely on a passed configuration value instead of global
variables, so that we can correctly call it in the directories that we
want.
@MrArnoldPalmer MrArnoldPalmer marked this pull request as ready for review February 21, 2023 22:50
@gitpod-io
Copy link

gitpod-io bot commented Feb 21, 2023

@aws-cdk-automation aws-cdk-automation requested a review from a team February 21, 2023 22:50
@github-actions github-actions bot added the p2 label Feb 21, 2023
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 21, 2023
Copy link
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.

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

reverting whitespace change to a file with no other changes, so that git history of most recent edit to this file will stay relevant
…git history of most recent edit to this file will stay relevant
…n the scope: inline the variable since its only used once
@MrArnoldPalmer MrArnoldPalmer added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Feb 22, 2023
@aws-cdk-automation aws-cdk-automation dismissed their stale review February 22, 2023 16:54

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

/**
* Map of CFN Scopes to modules
*/
scopeMap: Record<string, string[]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Using this type and the ModuleMap interface to represent almost the same thing I found really confusing. I think we should encapsulate the process of turning the json file into the ModuleMap type. It's a bit hard to explain in comments, so I'm going to push to the branch to describe what I mean.

Copy link
Contributor

Choose a reason for hiding this comment

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

See the commit here: d91b821

Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

I'm good to merge this now! As long as you are good with the ModuleMap changes @MrArnoldPalmer

@MrArnoldPalmer MrArnoldPalmer merged commit e8d526e into aws:feat/repo-restructure Feb 22, 2023
@MrArnoldPalmer MrArnoldPalmer deleted the feat/repo-restructure-refactor branch April 26, 2023 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants