all: migrate to modules #672
Conversation
gbbr
force-pushed
the
gbbr/mod
branch
10 times, most recently
from
cebbf36
to
f906abc
Compare
gbbr
force-pushed
the
gbbr/mod
branch
4 times, most recently
from
6ba924a
to
3b801c1
Compare
gbbr
force-pushed
the
gbbr/mod
branch
3 times, most recently
from
a8236dc
to
38ad1dd
Compare
gbbr
force-pushed
the
gbbr/mod
branch
7 times, most recently
from
a18cae3
to
ca4e130
Compare
gbbr
force-pushed
the
gbbr/mod
branch
3 times, most recently
from
24cb7c3
to
38115e9
Compare
go.mod
Outdated
| github.com/DataDog/datadog-go v3.7.1+incompatible | ||
| github.com/philhofer/fwd v1.0.0 // indirect | ||
| github.com/tinylib/msgp v1.1.2 | ||
| github.com/zenazn/goji v1.0.1 |
There was a problem hiding this comment.
It was an accident. There should be only one entry in this file: tinylib/msgp
There was a problem hiding this comment.
I think this is going to be weird. It requires doing:
git update-index --assume-unchanged ./go.mod
Because the file exists in the repo :|
.circleci/config.yml
Outdated
| environment: | ||
| GO111MODULE: "on" | ||
| GOPATH: "/home/circleci/go" | ||
| working_directory: /home/circleci/go/src/gopkg.in/DataDog/dd-trace-go.v1 |
There was a problem hiding this comment.
lets not have the module downloaded within gopath?
if a module has go.mod it should work from outside and that will be a good test for us to do
There was a problem hiding this comment.
Yeah that might be better.
| command: | | ||
| go run checkcopyright.go | ||
|
|
||
| - run: | ||
| name: Check gofmt | ||
| name: gofmt |
There was a problem hiding this comment.
on a step before this let us restore cache of go modules and and we can save below. this will make each run a lot faster
There was a problem hiding this comment.
Haven't figured out how to do that. Not sure how to get a good checksum. Tried to use go.sum but it's not reliable or I need to somehow update it. Ideas welcome.
.circleci/config.yml
Outdated
| docker: | ||
| - image: circleci/golang:1.13 | ||
| environment: | ||
| GO111MODULE: "on" | ||
| GOPATH: "/home/circleci/go" |
There was a problem hiding this comment.
lets move all docker to one executor and use there.
we have building on go1.12 and testing on go1.13. Might be better to have them all on the same version
There was a problem hiding this comment.
oh nvm i saw the conversation re different golang versions.
Still think listing the executors on top might be cool: https://circleci.com/docs/2.0/configuration-reference/#executors-requires-version-21
There was a problem hiding this comment.
Some of the contrib packages require go1.13 (k8s.io/client-go), but we would prefer to test at least the tracer (ddtrace/tracer) with the minimum supported version - go1.12
There was a problem hiding this comment.
Sorry @roopakv, somehow your second message didn't show up when I was responding. I only saw the first comment.
There was a problem hiding this comment.
Might be something to look into if it saves CI time and wasted resources. It's easy to get go1.12 using go get golang.org/dl/go1.12 just for the build. The base executor image could have both.
gbbr
force-pushed
the
gbbr/mod
branch
3 times, most recently
from
2595780
to
786cadc
Compare
There was a problem hiding this comment.
Thanks for the input @roopakv. I tried to apply some of it. Since the main goal of this PR was to switch to modules we'll go ahead and merge it and are happy to make more CI changes or improvements in follow up PRs.
The executor idea sounded interesting, we have quite a couple of jobs now using the same image go1.13, we should consider that.
This is the workflow:
.circleci/config.yml
Outdated
| environment: | ||
| GO111MODULE: "on" | ||
| GOPATH: "/home/circleci/go" | ||
| working_directory: /home/circleci/go/src/gopkg.in/DataDog/dd-trace-go.v1 |
There was a problem hiding this comment.
Yeah that might be better.
| command: | | ||
| go run checkcopyright.go | ||
|
|
||
| - run: | ||
| name: Check gofmt | ||
| name: gofmt |
There was a problem hiding this comment.
Haven't figured out how to do that. Not sure how to get a good checksum. Tried to use go.sum but it's not reliable or I need to somehow update it. Ideas welcome.
Migrate to go modules. The `go.mod` file is not committed into the repository and contains entries for dependencies exclusively needed by the repository and none of the integrations. This is so because we want the importing program to have the final word in deciding versions. As a side-effect, we were forced to update the import path of what was formerly `gopkg.in/Shopify/sarama.v1` to `github.com/Shopify/sarama`. For more details, please see: IBM/sarama#1510 Additionally, CI change were introduced which reduce time by about 50%.
One big module, no requirements besides
msgp. This allows the importing module to have the final word with import versions just like in the previous "no module" world.Shopify/saramausers will have to update to the repository's enforced import path in case they were using what was in the README (The gopkg.in import path can't be used with recent Go versions while the README file claims otherwise IBM/sarama#1510) like usCI now uses workflows. Seems to take around 3.5 minutes.