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

Add support for 'any resource' codegen #1336

Merged
merged 3 commits into from
Apr 20, 2024
Merged

Add support for 'any resource' codegen #1336

merged 3 commits into from
Apr 20, 2024

Conversation

AaronFriel
Copy link
Contributor

Part of pulumi/pulumi#6346.

Depends on commit in pulumi/pulumi#15793.

@AaronFriel AaronFriel requested review from t0yv0, justinvp and iwahbe March 27, 2024 01:50
@AaronFriel AaronFriel mentioned this pull request Mar 27, 2024
Merged
t0yv0
t0yv0 reviewed Mar 29, 2024
View reviewed changes
pkg/codegen/java/names/known.go
@@ -20,7 +20,7 @@ var PulumiInternal = PulumiCore.Dot("internal")

var PulumiAnnotations = PulumiCore.Dot("annotations")

var ResourceType = PulumiAnnotations.Dot("ResourceType")
var ResourceTypeAnnotation = PulumiAnnotations.Dot("ResourceType")
Copy link
Member

Choose a reason for hiding this comment

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

Very nice, less confusing.

@t0yv0 t0yv0 self-requested a review March 29, 2024 16:58
t0yv0
t0yv0 approved these changes Mar 29, 2024
View reviewed changes
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

This looks good, fairly surgical. For good form it would be nice to see a quick test and have the go.mod upgrade separate but I don't know how much friction there is in this repo. Thank you!

@AaronFriel AaronFriel requested a review from t0yv0 April 13, 2024 00:01
@AaronFriel
Copy link
Contributor Author

@t0yv0 @justinvp getting tests to pass required more changes than I expected, as it looks like SDK codegen has changed:

https://github.com/pulumi/pulumi-java/pull/1336/files#diff-5faecbaf7ae0e55d4c63d83d70d5144ba4fd1dcf5eadfd47260a3f5de57fab16

But tests are passing. I leave this up to you to review & decide if this is mergeable.

t0yv0
t0yv0 approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

LGTM but I don't do much in terms of maintenance on this repo these days.. Looks like a surgical change plus lots of go.mod update-related Chun in testdata.

@AaronFriel AaronFriel added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Apr 20, 2024
@AaronFriel AaronFriel merged commit 3dd8279 into main Apr 20, 2024
22 checks passed
@AaronFriel AaronFriel deleted the friel/any-resource branch April 20, 2024 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t0yv0 t0yv0 t0yv0 approved these changes

@justinvp justinvp Awaiting requested review from justinvp

@iwahbe iwahbe Awaiting requested review from iwahbe

Assignees
No one assigned
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AaronFriel @t0yv0