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

update to go modules #38

Merged
merged 4 commits into from
Oct 1, 2019
Merged

update to go modules #38

merged 4 commits into from
Oct 1, 2019

Conversation

mcristina422
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Sep 30, 2019

Coverage Status

Coverage remained the same at 21.833% when pulling 37601a4 on mcristina422:go113 into 929d044 on grafana-tools:master.

@grafov grafov self-assigned this Sep 30, 2019
@grafov
Copy link
Member

grafov commented Sep 30, 2019

@mcristina422 thank for your contribution.

  • dashboard-unmarshal_test.go was fixed earlier in a separate PR it is better to remove it from this PR for clarity.
  • I think we could use go modules but keep vendor directory with the packages yet — it is the safest way. If you could use modules you can build with them. Those who still use vendor libs could build without modules support. Go till 1.14 still uses GO111MODULE=auto so the build with modules is not a default way yet. cmd/go: default to GO111MODULE=auto (with changes) for Go 1.13 golang/go#31857 So I'll prefer to wait for future Go versions before totally remove vendored packages.

@mcristina422
Copy link
Contributor Author

Sure, I can add back the vendor folder. But in go1.13 auto means go modules are on by default. https://golang.org/doc/go1.13#modules

The GO111MODULE environment variable continues to default to auto, but the auto setting now activates the module-aware mode of the go command whenever the current working directory contains, or is below a directory containing, a go.mod file — even if the current directory is within GOPATH/src.

@grafov
Copy link
Member

grafov commented Oct 1, 2019

@mcristina422 thank for the clarification. Really modules are on by default now. Ok, let migrate with small steps. Accept changes, thank you for this contribution!

@grafov grafov merged commit 30644e5 into grafana-tools:master Oct 1, 2019
This was referenced Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants