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

Update go version in circleci workflows #28

Merged
merged 6 commits into from
Jan 7, 2022
Merged

Update go version in circleci workflows #28

merged 6 commits into from
Jan 7, 2022

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Jan 7, 2022 •
edited by robbkidd
Loading

Which problem is this PR solving?

Updates go to v1.17.

Short description of the changes

  • updates go default version to v1.17 in circleci workflows
  • drops testing against go 1.10 and 1.11
  • matrix'd tests for Go 1.12 - 1.17
  • updated to use Circle's "new" cimg/go images
  • migrated to Go modules
  • renamed the job and workflow to "test" because that's all we're doing here

@MikeGoldsmith MikeGoldsmith added version: bump patch A PR with release-worthy changes and is backwards-compatible. type: maintenance The necessary chores to keep the dust off. labels Jan 7, 2022
@MikeGoldsmith MikeGoldsmith self-assigned this Jan 7, 2022
@MikeGoldsmith MikeGoldsmith requested a review from a team January 7, 2022 16:24
@robbkidd
Copy link
Member

robbkidd commented Jan 7, 2022 •
edited
Loading

Go 10, 11, and 12 are being passed to the test_dynsampler job in the build_dynsampler workflow despite the new default of Go 17.

The attempt to test under an old Go version is failing because the CA certs in the CircleCi-provided Go images are old and don't know about the new certificate authority cert for gopkg.in issued February 2020. The same issue came up in libhoney-go. The fix there was to update the CA certs package during test so as to be able to continue testing under Go 1.12. We can either do that here as well, or drop testing on old Go versions.

@robbkidd
matrixify the versions of Go to test
95a1e7c 
Let's see which work!
@robbkidd
drop Go 1.10 and 1.11
bffaa16 
CircleCi doesn't have a cimg/go for 1.10 and 1.11's is unhappy running
sudo  to get updated cacerts. They're old. We should stop testing
against them.

Updated to matrixify like we do in libhoney-go.
@robbkidd
go mod, go!
5a204b3 
Targeted Go 1.12 to work our way up to something modern.
@robbkidd
Copy link
Member

robbkidd commented Jan 7, 2022

Migrated to Go modules to get Go 1.12 through 1.17 testing happily.

@robbkidd
just call these "test"
22ade91 
We know it's dynsampler already. Also, the workflow isn't really building
anything; it only runs the tests across multiple Go versions.
@robbkidd
Copy link
Member

robbkidd commented Jan 7, 2022

Latest change was to rename the job and the workflow to "test". We're in the dynsampler project, so we know that's what is being tested. The workflow only runs tests, so "build" had me thinking an artifact might come out.

Screen Shot 2022-01-07 at 16 19 55

What do y'all think?

@robbkidd robbkidd added status: review needed Changes need review. and removed status: blocked labels Jan 7, 2022
JamieDanielson
JamieDanielson approved these changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

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

🎉

robbkidd
robbkidd reviewed Jan 7, 2022
View reviewed changes
go.mod Show resolved Hide resolved
vreynolds
vreynolds reviewed Jan 7, 2022
View reviewed changes
.circleci/config.yml Outdated
default: "12"
working_directory: /go/src/github.com/honeycombio/dynsampler-go
default: *default_goversion
working_directory: /home/circleci/go/src/github.com/honeycombio/dynsampler-go
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think working directory is necessary. I removed it recently on another repo PR that updated the docker image

Copy link
Member

Choose a reason for hiding this comment

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

It was present here and in libhoney-go. But let's find out! 🧑‍🔬 :atom: ⚗️ 🧪

Copy link
Contributor

Choose a reason for hiding this comment

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

This was the PR: https://github.com/honeycombio/buildevents/pull/132/files

Copy link
Member

Choose a reason for hiding this comment

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

Nice!

robbkidd
robbkidd reviewed Jan 7, 2022
View reviewed changes
go.mod

go 1.12

require github.com/stretchr/testify v1.6.1
Copy link
Member

Choose a reason for hiding this comment

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

Noting here for posterity: this is the version that still works with Go 1.12. testify 1.7 requires Go 1.13.

@robbkidd robbkidd merged commit 993cc14 into main Jan 7, 2022
@robbkidd robbkidd deleted the mike/update-go branch January 7, 2022 22:44
@robbkidd robbkidd removed the status: review needed Changes need review. label Jan 7, 2022
@vreynolds vreynolds added version: no bump A PR with maintenance or doc changes that aren't included in a release. and removed version: bump patch A PR with release-worthy changes and is backwards-compatible. labels Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JamieDanielson JamieDanielson JamieDanielson approved these changes

@robbkidd robbkidd robbkidd left review comments

@vreynolds vreynolds vreynolds approved these changes

Assignees

@MikeGoldsmith MikeGoldsmith

Labels
type: maintenance The necessary chores to keep the dust off. version: no bump A PR with maintenance or doc changes that aren't included in a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update to cimg/go image and go 1.16.x
4 participants
@MikeGoldsmith @robbkidd @vreynolds @JamieDanielson