Skip to content

chore: move the build of the experimental modules to a separate script#15722

Merged
mergify[bot] merged 19 commits intoaws:v2-mainfrom
skinny85:chore/move-v2-experiments-build-to-script
Jul 30, 2021
Merged

chore: move the build of the experimental modules to a separate script#15722
mergify[bot] merged 19 commits intoaws:v2-mainfrom
skinny85:chore/move-v2-experiments-build-to-script

Conversation

@skinny85
Copy link
Copy Markdown
Contributor

@skinny85 skinny85 commented Jul 22, 2021

Instead of building the experimental modules as part of the main monorepo,
move their generation into a separate script,
mastered in scripts/transform.sh.


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

@skinny85 skinny85 requested a review from nija-at July 22, 2021 18:50
@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Jul 22, 2021

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jul 22, 2021
@skinny85 skinny85 force-pushed the chore/move-v2-experiments-build-to-script branch 2 times, most recently from 62e5903 to 3547469 Compare July 26, 2021 21:20
@skinny85 skinny85 requested a review from nija-at July 26, 2021 23:07
@skinny85
Copy link
Copy Markdown
Contributor Author

@nija-at included all your comments, would appreciate a re-review!

@nija-at
Copy link
Copy Markdown
Contributor

nija-at commented Jul 27, 2021

@nija-at included all your comments, would appreciate a re-review!

I'm out for the day today.
Tried to do a quick review but unfortunately, I'll need to take a look again properly when I'm back in the office tomorrow. Sorry for the delay.

Copy link
Copy Markdown
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Sorry it took longer to review this. I had to deep dive into the scripts a little to provide useful feedback.

@skinny85 skinny85 force-pushed the chore/move-v2-experiments-build-to-script branch 3 times, most recently from a8fa253 to 3e4a464 Compare July 29, 2021 00:42
@skinny85 skinny85 force-pushed the chore/move-v2-experiments-build-to-script branch from a2a2476 to 1a4b838 Compare July 29, 2021 00:58
@skinny85 skinny85 requested a review from nija-at July 29, 2021 01:38
@skinny85
Copy link
Copy Markdown
Contributor Author

@nija-at thanks for the review. Included all of your comments, would appreciate a re-review!

Copy link
Copy Markdown
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Looking much better now. Still a few more things.

Please take a look at my responses from the previous review. I've left the comments that need addressing as 'unresolved'.

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: b7f2687
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@skinny85
Copy link
Copy Markdown
Contributor Author

@nija-at incorporated all of your comments from the last review, should be good to go!

@nija-at nija-at added the pr/do-not-merge This PR should not be merged at this time. label Jul 30, 2021
@skinny85 skinny85 removed the pr/do-not-merge This PR should not be merged at this time. label Jul 30, 2021
@mergify mergify bot merged commit 7ae7518 into aws:v2-main Jul 30, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jul 30, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@skinny85 skinny85 deleted the chore/move-v2-experiments-build-to-script branch July 30, 2021 18:04
njlynch added a commit that referenced this pull request Aug 3, 2021
As part of #15722, the jsii targets for experimental packages on the v2 branch
were changed to include an 'alpha' identifier so the package and module names
would more obviously reflect the stability of the packages.

However, since this change was made directly to the package.json files, this
means that all usages of these targets across the source were impacted,
including usages via ubergen. The impact of this is that the L1s published to
aws-cdk-lib were incorrectly published under this 'alpha' module instead of
their normal location. L1s should still go under the standard location, as they
are stable; only the experimental L2s should be impacted by the alpha
designator.

fixes #15845
njlynch added a commit that referenced this pull request Aug 4, 2021
…mation

As part of #15722, the decision was made to change the package.json files for
experimental packages to add 'alpha' identifiers. This ends up causing issues
for our ubergen-ed packages (i.e., aws-cdk-lib), which then try to include L1s
for these packages under the 'alpha' namespaces. Instead, switch (back) to
adding the alpha identifiers as part of the package transformation step.

Reverted the change to the pkglint rules, and re-ran `yarn pkglint` to update
the package.json files.

I also did some light refactoring, since the method was a bit hard to read.

fixes #15845
mergify bot pushed a commit that referenced this pull request Aug 4, 2021
…mation (#15890)

As part of #15722, the decision was made to change the package.json files for
experimental packages to add 'alpha' identifiers. This ends up causing issues
for our ubergen-ed packages (i.e., aws-cdk-lib), which then try to include L1s
for these packages under the 'alpha' namespaces. Instead, switch (back) to
adding the alpha identifiers as part of the package transformation step.

Reverted the change to the pkglint rules, and re-ran `yarn pkglint` to update
the package.json files.

I also did some light refactoring, since the method was a bit hard to read.

fixes #15845

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants