Upgrade go compiler 1.13.4 -> 1.14.3 #6359
Conversation
ghost
requested a review
from
| go test -i $(TEST) || exit 1 | ||
| echo $(TEST) | \ | ||
| xargs -t -n4 go test $(TESTARGS) -timeout=30s -parallel=4 | ||
| go test $(TESTARGS) -timeout=30s $(TEST) |
There was a problem hiding this comment.
There's no need to pipe the list of packages to . I was told parallelism was being limited to 4 for CI reasons, but I'm skeptical of that. Unless you have some very weird CI situation, limiting parallelism just slows down your tests a little. If a higher parallelism causes test failures, then you've got race conditions in your tests and they should be fixed or xargsparallelism should be set to 1.
| @@ -24,4 +24,4 @@ require ( | |||
| google.golang.org/api v0.22.0 | |||
| ) | |||
|
|
|||
| go 1.13 | |||
| go 1.14 | |||
There was a problem hiding this comment.
This line doesn't prevent recent Go 1.13 compilers from building the plugin. It just says "use the latest modules behavior." Keeping this up-to-date can prevent some dependency headaches.
|
Similarly handing off to @megan07 - related to the vendor removal work. :) |
|
It is very likely we will want to block this on the removal of |
|
@ndmckinley Is that going to merge soon? EDIT: I see that #6368 merged. I've updated this PR to remove the |
|
Great. I'm not 100% sure whether this is going to require a change to magic-modules - I think probably not - but investigating that will take a while, and we'll also need to copy this change to the beta provider. Is this urgent for you? If not, we'll probably schedule that work through our normal sprint planning process. |
|
Nope. This is not urgent. It's just a stepping stone to some other PRs I'd like to submit. |
|
Great, in that case I will let this get triaged and prioritized the normal way! |
Also simplify `make test` command. Remove `GO111MODULE=on` and `GOFLAGS=-mod=vendor` env vars, which are no longer necessary in Go 1.14.
ryboe
changed the title
There was a problem hiding this comment.
Hi @ryboe! Sorry for the delay, we had some other prioritized work ahead of this. But this looks good, thanks for contributing! Would you like to make the same changes to https://github.com/terraform-providers/terraform-provider-google-beta? If you don't have the time, that's fine, please let me know and I will make those changes for you.
Once the changes are in terraform-provider-google-beta as well I will merge this one and take a look at the other PR.
Thanks again!
|
Thanks, can you please apply this to |
| @@ -2,15 +2,13 @@ TEST?=$$(go list ./...) | |||
| WEBSITE_REPO=github.com/hashicorp/terraform-website | |||
| PKG_NAME=google | |||
|
|
|||
| GO111MODULE=on | |||
There was a problem hiding this comment.
I'm so sorry. As I'm making these changes in beta, I saw this and I wonder if it does any harm leaving this in? I'm thinking in case the current environment already has it set, it might be nice to have this here to override it specifically for this makefile. What do you think?
There was a problem hiding this comment.
Hmm, I don't think there are too many people who have GO111MODULE=off in their shell configs. That would break too many things. The only time I've seen in set to off is when you're want the old go get behavior, e.g.
GO111MODULE=off go get github.com/foo/bar/cmd/bazI could be wrong, though.
My preference is to embrace the new default behavior, and let people disable it inline (like the snippet above), but I'll leave it up to you.
There was a problem hiding this comment.
Ok! We'll try it and if we get any feedback we can always add it back in. That sounds find to me! Thanks!
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
goto version 1.14.3make testcommandGO111MODULE=onandGOFLAGS=-mod=vendorenv vars, which are no longer necessary in Go 1.14GO111MODULE=onis the default behavior nowgo buildandgo testto usevendor/. It will usevendor/if the directory is presentThis is a resubmit of #6223.