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

fix(hcl2cdk): only run get conditionally #846

Merged
merged 3 commits into from
Jul 29, 2021
Merged

Conversation

DanielMSchmidt
Copy link
Contributor

Should fix running get if there are no modules and providers added to the cdktf.json

@DanielMSchmidt DanielMSchmidt requested a review from jsteinich July 26, 2021 18:46
Comment on lines 148 to 150
const { terraformModules, terraformProviders } = JSON.stringify(
cdktfJson
) as any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just check the object directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean config.terraformModules etc?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean why convert cdktfJson to a string when it is already an object?

@DanielMSchmidt DanielMSchmidt force-pushed the check-before-cdktf-get branch 4 times, most recently from e8a7179 to 143009e Compare July 28, 2021 12:58

Verified

This commit was signed with the committer’s verified signature.
snyk-bot Snyk bot
@DanielMSchmidt DanielMSchmidt force-pushed the check-before-cdktf-get branch from 143009e to e685b5a Compare July 28, 2021 13:36
Comment on lines +305 to +306
terraformProviders: any[];
terraformModules: any[];
Copy link
Member

Choose a reason for hiding this comment

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

nit: they could also be undefined / not set for the input, but they won't be when they're outputted, right?

@DanielMSchmidt DanielMSchmidt merged commit 79324c6 into main Jul 29, 2021
@DanielMSchmidt DanielMSchmidt deleted the check-before-cdktf-get branch July 29, 2021 14:09
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants