-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[codegen/go] Simplify object default applies. #8539
Conversation
The overall goal of this change is to avoid unnecessary conversions between pointer and non-pointer output types. The use of these conversions requires that we always generate the pointer variant of an object type, which impedes #7943's ability to omit these types. Instead, we can let the return type of the callback match the argument type and return the dereferenced result of `foo.Defaults()` if necessary. These changes also simplify the generated code somewhat in order to avoid introducing additional variables.
Diff for pulumi-azuread with merge commit 427f556 |
Diff for pulumi-random with merge commit 427f556 |
Diff for pulumi-random with merge commit 92c0d10 |
Diff for pulumi-azuread with merge commit 92c0d10 |
Diff for pulumi-kubernetes with merge commit 427f556 |
Diff for pulumi-kubernetes with merge commit 92c0d10 |
Diff for pulumi-gcp with merge commit 92c0d10 |
Diff for pulumi-gcp with merge commit 427f556 |
Diff for pulumi-azure with merge commit 92c0d10 |
Diff for pulumi-azure with merge commit 427f556 |
Diff for pulumi-aws with merge commit 92c0d10 |
Diff for pulumi-aws with merge commit 427f556 |
Codecov Report
@@ Coverage Diff @@
## master #8539 +/- ##
==========================================
- Coverage 58.63% 58.62% -0.01%
==========================================
Files 634 634
Lines 96865 96844 -21
Branches 1378 1378
==========================================
- Hits 56794 56779 -15
+ Misses 36808 36799 -9
- Partials 3263 3266 +3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 nits, Otherwise LGTM.
- I would love to see some comments describing
outputType
andoutputTypeImpl
(and why the both exist). - My understanding is that this cleans up only our internal implementation of
Defaults
. Do we need a changelog entry for this? (I'm genuinely asking. I've heard conflicting definitions of "changelog worthy)
Diff for pulumi-azure-native with merge commit 427f556 |
Diff for pulumi-azure-native with merge commit 92c0d10 |
These days I think that we should be adding CL entries for all codegen changes, especially to help @pulumi/platform-integrations and @pulumi/native-providers understand that sort of changes to expect in the generated SDKs. |
Done. |
Diff for pulumi-azuread with merge commit 3ec6d3f |
Diff for pulumi-random with merge commit 3ec6d3f |
Diff for pulumi-kubernetes with merge commit 3ec6d3f |
Diff for pulumi-gcp with merge commit 3ec6d3f |
Diff for pulumi-azure with merge commit 3ec6d3f |
Diff for pulumi-aws with merge commit 3ec6d3f |
Diff for pulumi-azure-native with merge commit 3ec6d3f |
The overall goal of this change is to avoid unnecessary conversions
between pointer and non-pointer output types. The use of these
conversions requires that we always generate the pointer variant of an
object type, which impedes #7943's ability to omit these types. Instead,
we can let the return type of the callback match the argument type and
return the dereferenced result of
foo.Defaults()
if necessary.These changes also simplify the generated code somewhat in order to
avoid introducing additional variables.