Skip to content
This repository has been archived by the owner on Jul 18, 2023. It is now read-only.

253 Add Java & YAML support #256

Closed
wants to merge 3 commits into from

Conversation

Asamsig
Copy link
Contributor

@Asamsig Asamsig commented Jul 1, 2022

#253

Updated pulumi-terraform-bridge version to enable Java and YAML support

@RobbieMcKinstry
Copy link
Contributor

Hey there @Asamsig ! Thanks for this PR! Would you mind adding an entry in the CHANGELOG for your PR?

@RobbieMcKinstry
Copy link
Contributor

RobbieMcKinstry commented Jul 8, 2022

It looks like bumping the version here resulted in a breaking change in CI. Did you manage to test this locally and get it to work? Thank you again for your contribution. :)

@Asamsig
Copy link
Contributor Author

Asamsig commented Jul 8, 2022

I'll take a look at the changelog.

Hmm, I'll take another look, I was able to verify Java and Yaml worked, but perhaps it broke something I didn't test.

Ran `go mod tidy -go=1.16 && go mod tidy -go=1.17` to specify indirect dependency tree
Updated GIthub Actions scripts to use Go 1.17 to build the project
@Asamsig Asamsig force-pushed the 253-add-java--yaml-support branch from 8176682 to e6a8274 Compare July 8, 2022 18:38
@Asamsig
Copy link
Contributor Author

Asamsig commented Jul 8, 2022

Seems that the problem was that Pulumi-yaml requires Go 1.17, and Github actions was only using 1.16.

@RobbieMcKinstry
Copy link
Contributor

RobbieMcKinstry commented Jul 11, 2022

Hmm, I wonder if this failure related to pulumi/pulumi-docker-containers#98

If so, a workaround is to have a Pulumi employee push this changeset to a branch and running CI there instead of from the fork.

@@ -1,22 +1,212 @@
module github.com/pulumi/tf2pulumi

go 1.16
go 1.17
Copy link
Contributor

Choose a reason for hiding this comment

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

@Asamsig I think you need to run go mod tidy on the repo to clean this file up. I could be wrong though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually did run go mod tidy, if you look in pulumi-terraform-bridge, it has gotten a similar massive indirect dependency tree. 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay, well, it is what it is then! 😅 👍🏻

Copy link
Contributor

@RobbieMcKinstry RobbieMcKinstry left a comment

Choose a reason for hiding this comment

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

This LGTM. I'm unsure if there's a restriction keeping us from bumping the go version to v1.17 however. I'm going to defer to the code owners to give the final approval.

@RobbieMcKinstry RobbieMcKinstry requested a review from a team July 11, 2022 15:33
@iwahbe iwahbe added the resolution/duplicate This issue is a duplicate of another issue label Jul 22, 2022
@iwahbe
Copy link
Member

iwahbe commented Jul 22, 2022

Merged as #260

@iwahbe iwahbe closed this Jul 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement Improvements or new features language/java language/yaml resolution/duplicate This issue is a duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants