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

Go Modules #2541

Merged
merged 2 commits into from
Jul 30, 2019
Merged

Go Modules #2541

merged 2 commits into from
Jul 30, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Jul 24, 2019

No description provided.

@codecov
Copy link

codecov bot commented Jul 24, 2019

Codecov Report

Merging #2541 into master will increase coverage by 0.02%.
The diff coverage is 33.33%.

Impacted Files Coverage Δ
pkg/skaffold/build/gcb/cloud_build.go 0% <0%> (ø) ⬆️
pkg/skaffold/deploy/labels.go 16.04% <0%> (ø) ⬆️
pkg/skaffold/kubernetes/watcher.go 100% <100%> (ø) ⬆️
pkg/skaffold/kubernetes/wait.go 45.83% <50%> (-0.22%) ⬇️

@dgageot dgageot changed the title Continuing work on Go Modules [WIP] Continuing work on Go Modules Jul 25, 2019
@nkubala
Copy link
Contributor

nkubala commented Jul 29, 2019

just tested the release build locally and it works for me, so looks like setting GOMODULE11=on in the makefile was enough to solve the xgo issue. @dgageot what more do you think needs to be done here?

@dgageot
Copy link
Contributor Author

dgageot commented Jul 29, 2019

@nkubala I think we're good. I was wondering if we should vendor dependencies or re-download them on each build. I think vendoring is faster. Let me remove the [WIP] prefix!

@dgageot dgageot changed the title [WIP] Continuing work on Go Modules Continuing work on Go Modules Jul 29, 2019
@dgageot dgageot changed the title Continuing work on Go Modules Go Modules Jul 29, 2019
@nkubala
Copy link
Contributor

nkubala commented Jul 29, 2019

I'm not 100% sure but I think if we're keeping the vendored deps then we want to pass -mod=vendor to go build? otherwise won't it fetch the deps from go.mod and use those to build instead?

@dgageot
Copy link
Contributor Author

dgageot commented Jul 29, 2019

@nkubala TIL. It's fixed now

@nkubala
Copy link
Contributor

nkubala commented Jul 29, 2019

cool. and because of this we don't need to worry about downloading from a proxy right?

@dgageot
Copy link
Contributor Author

dgageot commented Jul 29, 2019 via email

nkubala
nkubala previously approved these changes Jul 29, 2019
Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

lets give it a shot

@nkubala
Copy link
Contributor

nkubala commented Jul 29, 2019

0.09s$ go build -mod=vendor -o out/skaffold.exe cmd/skaffold/skaffold.go
The command "go build -mod=vendor -o out/skaffold.exe cmd/skaffold/skaffold.go" exited with 1.

390.10s$ go test -short -timeout 60s ./...
build flag -mod=vendor only valid when using modules

not sure why this is happening, or why it only happened with the windows build...

@nkubala nkubala dismissed their stale review July 29, 2019 21:12

looks like the windows build is broken, let's figure that out

@dgageot dgageot force-pushed the go-modules branch 5 times, most recently from 0e6ab6d to 24708b2 Compare July 30, 2019 14:44
Signed-off-by: David Gageot <[email protected]>
Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

everything is green! @dgageot merge when you're ready

@dgageot dgageot merged commit bf847ea into GoogleContainerTools:master Jul 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants