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.

Sign up for GitHub

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

Jump to bottom

chore(lib): BREAKING CHANGE upgrade constructs lib to v10 #921

Merged
merged 22 commits into from Aug 26, 2021
Merged

chore(lib): BREAKING CHANGE upgrade constructs lib to v10 #921

merged 22 commits into from Aug 26, 2021

Conversation

ansgarm
Copy link
Member

@ansgarm ansgarm commented Aug 18, 2021 •
edited

This upgrades the constructs lib to v10 which is used by AWS CDK v2 and is supposed to be stable for a long time.

It also removed some functionality (e.g. Aspects, onSynthesize, onPrepare) which therefore is going to be included in the cdktf package.

Todos

  • Add Aspect to cdktf package
  • Test for Aspect
  • Validate all child constructs as well and re-add source property
  • Print info, warn and error Annotations in CLI
  • Add tests for CLI info printing
  • Let cdktf synth fail if error Annotations are encountered and no env flag is set (CLI will set the flag to be able to handle errors itself)
  • Write upgrade guide (in case users used e.g. onPrepare, Aspects import changes)

@DanielMSchmidt
Copy link
Contributor

@ansgarm I think we don't release through semantic-release yet, but just so you are warned for the future: a BREAKING CHANGE anywhere in the commit message will trigger a major release in semantic-release. Ran into this too often :D

@ansgarm ansgarm force-pushed the upgrade-constructs branch 2 times, most recently from 03a2133 to a5eeb9e Compare August 20, 2021 11:11
skorfmann
skorfmann reviewed Aug 20, 2021
View reviewed changes
packages/cdktf/lib/terraform-asset.ts Outdated Show resolved Hide resolved
packages/cdktf/test/stack.test.ts Show resolved Hide resolved
packages/cdktf/lib/app.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/app.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/app.ts Outdated Show resolved Hide resolved
packages/cdktf/lib/synthesize/synthesizer.ts Show resolved Hide resolved
@ansgarm
chore(lib): BREAKING CHANGE upgrade constructs lib to v10
b90f8ba
@ansgarm
chore(examples): update constructs to v10
00c47d2
@ansgarm
chore: update constructs to v10 in more places
a071edf
@ansgarm
chore: update constructs to v10 in more places
815c864
@ansgarm
feat(lib): add Aspect and Annotations directly to cdktf as constructs…
64a9a2b 
… v10 removed them
@ansgarm
chore(lib): fix tests and lint
00eb820
@ansgarm
test(lib): add test for Manifest containing Annotations
49f5bb7
@ansgarm
fix(lib): stub version in Manifest aswell
4ee691c
@ansgarm
feat(lib): Add renderConstructTree to cdktf Testing utility
193a7a6
@ansgarm
test(lib): add test for Aspects
778c5a3
@ansgarm
test(cli): fix tests after adding Annotations to Manifest
0a70c38
@ansgarm
feat(cli): Print Annotations from synth
67d2c66 
also addressed review feedback for a first part
@ansgarm
feat(lib): throw an error when synth encounters error Annotations
6530db6
@ansgarm
test(lib): fix test for manifest annotations
5b4c3c0
@ansgarm
feat(lib): add internal support for custom synthesis
ad38d78
@ansgarm
fix(lib): copy asset files on synth and not on creation (constructor)
81166d4
@ansgarm
chore(tests): add integration test for annotations
d7c6762
@ansgarm
chore(docs): add docs for breaking constructs upgrade
dd5d19a
@ansgarm ansgarm marked this pull request as ready for review August 24, 2021 14:09
@ansgarm ansgarm requested a review from skorfmann August 24, 2021 14:09
skorfmann
skorfmann approved these changes Aug 25, 2021
View reviewed changes
Copy link
Contributor

@skorfmann skorfmann left a comment

Choose a reason for hiding this comment

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

Just skimmed over it to see what changed compared to the last review. Looks pretty good from my point of view.

DanielMSchmidt
DanielMSchmidt approved these changes Aug 25, 2021
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
packages/cdktf/lib/app.ts Show resolved Hide resolved
packages/cdktf/lib/manifest.ts Show resolved Hide resolved
packages/cdktf/lib/synthesize/synthesizer.ts Show resolved Hide resolved
packages/cdktf/lib/testing.ts Show resolved Hide resolved
@ansgarm ansgarm enabled auto-merge August 26, 2021 09:26
@ansgarm ansgarm merged commit f0ffd60 into main Aug 26, 2021
@ansgarm ansgarm deleted the upgrade-constructs branch August 26, 2021 09:58
@skorfmann skorfmann mentioned this pull request Aug 26, 2021
Closed
@skorfmann skorfmann mentioned this pull request Sep 15, 2021
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jsteinich jsteinich jsteinich left review comments

@skorfmann skorfmann skorfmann approved these changes

@DanielMSchmidt DanielMSchmidt DanielMSchmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ansgarm @DanielMSchmidt @skorfmann @jsteinich