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

Fix nodejs external enums #10286

Merged
merged 7 commits into from Jul 29, 2022
Merged

Fix nodejs external enums #10286

merged 7 commits into from Jul 29, 2022

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Jul 29, 2022 •
edited

Description

Fixes #10253

I fixed a couple of problems in this PR, mostly so the test that I added would pass without modification.

Bugs fixed:

  • Python generated invalid code when it had only empty submodules
  • Node didn't add the index.ts to the types folder unless a object style type was generated in that package. I changed it to always generate the index.ts, allowing bare enums to be referenceable. This fixed the compile error for the plain-and-default test.
  • We didn't accommodate that enums could be external types. We now fix the problem.

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • Yes, there are changes in this PR that warrants bumping the Pulumi Service API version

@iwahbe iwahbe marked this pull request as draft July 29, 2022 10:42
@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@iwahbe iwahbe added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Jul 29, 2022
@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@iwahbe iwahbe requested review from Frassle and Zaid-Ajaj July 29, 2022 13:28
@iwahbe iwahbe self-assigned this Jul 29, 2022
@iwahbe iwahbe marked this pull request as ready for review July 29, 2022 13:28
@iwahbe iwahbe removed the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Jul 29, 2022
@pulumi-bot
Copy link
Contributor

Please view the results of the Downstream Codegen Tests Here

@iwahbe
Copy link
Member Author

iwahbe commented Jul 29, 2022

The 5 failing downstream tests are due to an unrelated interface change in workspace.GetPluginPath.

@iwahbe
Copy link
Member Author

iwahbe commented Jul 29, 2022

The last running test is known to run indefinitely. Given that the test is for python, and unrelated to the codegen change, I'm going to merge.

@iwahbe iwahbe merged commit 60481da into master Jul 29, 2022
@pulumi-bot pulumi-bot deleted the iwahbe/10253/nodejs-external-enums branch July 29, 2022 15:07
@iwahbe iwahbe mentioned this pull request Aug 11, 2022
Closed
@julienp julienp mentioned this pull request Dec 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Frassle Frassle Frassle approved these changes

@Zaid-Ajaj Zaid-Ajaj Awaiting requested review from Zaid-Ajaj

Assignees

@iwahbe iwahbe

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[codegen/nodejs] Invalid Type Declaration for Foreign Enums
3 participants
@iwahbe @pulumi-bot @Frassle