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

Upgrade to Go 1.18 #232

Merged
merged 6 commits into from
May 23, 2022
Merged

Upgrade to Go 1.18 #232

merged 6 commits into from
May 23, 2022

Conversation

jukie
Copy link
Collaborator

@jukie jukie commented May 16, 2022 •
edited
Loading

Bumping the project to go 1.18
Tested locally and things are functioning still.

yermulnik
yermulnik reviewed May 17, 2022
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
yermulnik
yermulnik reviewed May 17, 2022
View reviewed changes
.circleci/config.yml Outdated Show resolved Hide resolved
.circleci/config.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@jukie jukie requested a review from yermulnik May 17, 2022 19:25
yermulnik
yermulnik approved these changes May 17, 2022
View reviewed changes
Copy link
Collaborator

@yermulnik yermulnik left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
@warrensbox Please review when you get a chance to.

jukie added a commit to jukie/terraform-switcher that referenced this pull request May 22, 2022
@jukie
Include changes from warrensbox#232: Upgrade to Go 1.18
19d4d80
@jukie
Copy link
Collaborator Author

jukie commented May 23, 2022

Following up from @warrensbox's email. @yermulnik are you able to merge this?
If so I could start splitting my large PR into smaller chunks and moving forward can work with you given Warren's availability.

@yermulnik
Copy link
Collaborator

@yermulnik are you able to merge this?

Yep, I do. And am going to w.r.t. this PR. Gimme a min.

If so I could start splitting my large PR into smaller chunks and moving forward can work with you given Warren's availability.

Sounds good for me. Though since I'm not quite good with Golang, I'd prefer a 2nd opinion for Golang bits. Same for large PRs as sometimes I don't have enough time and expertise to deep dive into details 😢

@yermulnik yermulnik merged commit 1b35962 into warrensbox:master May 23, 2022
@yermulnik
Copy link
Collaborator

@jukie Any idea why this broke CircelCI build? https://app.circleci.com/pipelines/github/warrensbox/terraform-switcher/408/workflows/c20ce45c-f4ef-4dbf-891f-d75870362edb/jobs/1225

I'm going to revert 1b35962 to see whether this is going to fix the build.

@jukie
Copy link
Collaborator Author

jukie commented May 23, 2022

@yermulnik the cimg/go image switched directories around and the default user can't create a directory under root (i.e. /go) so checkout is failing.
I'll reopen with a change in working directory to the new default at /home/circleci/go.

@yermulnik
Copy link
Collaborator

@jukie Thanks. I'm still online so can merge once it's ready.

@jukie jukie mentioned this pull request May 23, 2022
Merged
@jukie
Copy link
Collaborator Author

jukie commented May 23, 2022

@yermulnik Added here: #235

warrensbox
warrensbox reviewed May 23, 2022
View reviewed changes
.circleci/config.yml
@@ -5,7 +5,7 @@ version: 2.1
jobs:
build:
docker:
- image: circleci/golang:1.16
- image: cimg/go:1.18

working_directory: /go/src/github.com/warrensbox/terraform-switcher
Copy link
Owner

Choose a reason for hiding this comment

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

The working directory is not right here:
In the previous version, it was /go/ for image: circleci/golang:1.16

It should be /home/circleci/go for the new image: cimg/go:1.18

yermulnik added a commit that referenced this pull request May 24, 2022
@jukie @yermulnik @warrensbox
Rebase over master (#237)
032e396 
* Automate binary tests from test-data directory (#234)

* Upgrade to Go 1.18 (#232)

* Upgrade to Go 1.18

* Fix image ref

* Fix git endpoint

* Fix CircleCI workflow

* Update Makefile

Co-authored-by: George L. Yermulnik <[email protected]>

* Fix makefile

Co-authored-by: George L. Yermulnik <[email protected]>

* Upgrade to Go 1.18

* Fix image ref

* Fix working directory

Co-authored-by: George L. Yermulnik <[email protected]>
Co-authored-by: Warren Veerasingam <[email protected]>
davidvasandani pushed a commit to davidvasandani/terraform-switcher that referenced this pull request Oct 26, 2022
@jukie @yermulnik @davidvasandani
Upgrade to Go 1.18 (warrensbox#232)
97cfe97 
* Upgrade to Go 1.18

* Fix image ref

* Fix git endpoint

* Fix CircleCI workflow

* Update Makefile

Co-authored-by: George L. Yermulnik <[email protected]>

* Fix makefile

Co-authored-by: George L. Yermulnik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@warrensbox warrensbox warrensbox left review comments

@yermulnik yermulnik yermulnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jukie @yermulnik @warrensbox