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

Add recommendation for scalar output to v3 migration guide #1448

Merged
merged 3 commits into from
Oct 17, 2024

Conversation

flostadler
Copy link
Contributor

#1445 and #1446 introduced new scalar properties as a workaround to the breaking Node.js SDK changes.

This documents those in the migration guide.

@flostadler flostadler added the needs-release/major Marking a PR to compute the next major version label Oct 16, 2024
@flostadler flostadler requested review from t0yv0 and a team October 16, 2024 19:01
@flostadler flostadler self-assigned this Oct 16, 2024
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

All properties of components in the Node.js SDK are now wrapped in `Output`. This aligns it with other language SDKs but introduces some breaking changes for optional resource properties.
Accessing those optional resource outputs now requires using `apply`.

To work around this we've exposed the most commonly used resource properties at the top level of the components. This change allows for easier access without requiring to use `apply`.
Copy link
Member

Choose a reason for hiding this comment

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

We have

@@ -167,6 +202,11 @@ The `NodeGroup` and `NodeGroupV2` components now accept inputs for the following

If you are using Go you will need to adjust your program to handle those types being inputs.

#### Security group and ingress rule inputs
We've updated the `NodeGroup` and `NodeGroupV2` components to allow passing in `nodeSecurityGroup` and `clusterIngressRule` by their IDs. You can now use `nodeSecurityGroupId` and `clusterIngressRuleId` as input properties.
Copy link
Member

Choose a reason for hiding this comment

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

One more we have

All properties of components in the Node.js SDK are now wrapped in `Output`. This aligns it with other language SDKs but introduces some breaking changes for optional resource properties.
Accessing those optional resource outputs now requires using `apply`.

To work around this we've exposed the most commonly used resource properties at the top level of the components. This change allows for easier access without requiring to use `apply`.
Copy link
Member

Choose a reason for hiding this comment

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

There's great information in this paragraph but it is a little verbose, try to do a pass and remove words that can be removed, otherwise it is reasonable.

Perhaps instead of "To work around this", "We added convenient access properties for frequently used outputs:".

@flostadler flostadler merged commit 726ccbf into release-3.x.x Oct 17, 2024
34 checks passed
@flostadler flostadler deleted the flostadler/v3-migration-guide-update branch October 17, 2024 14:08
flostadler added a commit that referenced this pull request Oct 17, 2024
#1445 and
#1446 introduced new scalar
properties as a workaround to the breaking Node.js SDK changes.

This documents those in the migration guide.
flostadler added a commit that referenced this pull request Oct 17, 2024
#1445 and
#1446 introduced new scalar
properties as a workaround to the breaking Node.js SDK changes.

This documents those in the migration guide.
flostadler added a commit that referenced this pull request Oct 17, 2024
#1445 and
#1446 introduced new scalar
properties as a workaround to the breaking Node.js SDK changes.

This documents those in the migration guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-release/major Marking a PR to compute the next major version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants