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

Upgrading go version from 1.17 to 1.18.3 #96

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

mikolajpe
Copy link

@mikolajpe mikolajpe commented Jul 4, 2022

This PR bumps following images:
go/Dockerfile, go/Dockerfile.ubi and pulumi/Dockerfile which build successfully,
along go version upgrade in .github workflow

https://tip.golang.org/doc/go1.18#:~:text=Go%201.18%20includes%20an%20implementation,compatible%20%2D%20changes%20to%20the%20language.

Go 1.18 includes an implementation of generic features as described by the Type Parameters Proposal. This includes major - but fully backward-compatible - changes to the language.

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.

Awesome! Looks good to me! Looks like tests are failing, however. Not sure what's going on there.

@mikolajpe
Copy link
Author

Awesome! Looks good to me! Looks like tests are failing, however. Not sure what's going on there.

@RobbieMcKinstry tests are failing as they are missing PULUMI_ACCESS_TOKEN

containers_test.go:36: PULUMI_ACCESS_TOKEN not found, aborting tests.

this is not related to the change I made

@RobbieMcKinstry
Copy link
Contributor

Yes, I agree, it's very plausible that the failing builds are unrelated to the changes you made. However, I'd like to get a better idea of why they're failing before merging since we don't want to break the build on the default branch.

@RobbieMcKinstry
Copy link
Contributor

@mikolajprzybysz would it be okay with you if I closed this PR in lieu of #97 ? My understanding is that Secrets are not accessible to PRs from forked repositories. If that's truly the case, I'm not sure how we haven't encountered this sooner. I'm going to open an issue to inspect and address.

In the meantime, we can get your changes in by using a branch PR instead of a fork PR, if that's okay with you! :)

@mikolajpe
Copy link
Author

@mikolajprzybysz would it be okay with you if I closed this PR in lieu of #97 ? My understanding is that Secrets are not accessible to PRs from forked repositories. If that's truly the case, I'm not sure how we haven't encountered this sooner. I'm going to open an issue to inspect and address.

In the meantime, we can get your changes in by using a branch PR instead of a fork PR, if that's okay with you! :)

@RobbieMcKinstry fine by me! I am for anything that helps merge this change in :)

@RobbieMcKinstry RobbieMcKinstry merged commit 9192185 into pulumi:main Jul 11, 2022
@RobbieMcKinstry
Copy link
Contributor

Thanks very much for understanding! I opened #98 to track the cause of the failed CI checks. :)

@mikolajpe
Copy link
Author

@RobbieMcKinstry could you release it as new version as well, I would like to use it to update pulumi-kubernetes-operator as well

@mikolajpe mikolajpe deleted the go1.18-upgrade branch July 11, 2022 17:01
@RobbieMcKinstry
Copy link
Contributor

Hi again! I see that the release process for this repo is a little different from our other repos. Let me check with the team and get back to you :)

@RobbieMcKinstry
Copy link
Contributor

Hey there @mikolajprzybysz ! Today I learned that this repository is released in lock-step with the pulumi CLI.
During the release process for the CLI, we build these Docker images and release them to the image repositories with tags that mirror the CLI release.

That is to say, you can expect this PR to make its way into v3.36.0 which is expected to release later this week. I will update the README with this information.

@mikolajpe
Copy link
Author

@RobbieMcKinstry thank you so much for the clarification, that's all I need to know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants