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

Terraform 0.12.0 lint #177

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Terraform 0.12.0 lint #177

wants to merge 11 commits into from

Conversation

morancj
Copy link
Member

@morancj morancj commented Aug 31, 2021

@morancj
Copy link
Member Author

morancj commented Aug 31, 2021

https://github.com/FiloSottile/gvt is archived: if someone more familiar with Go than I would kindly TAL, hopefully we can get these tests passing. It seems this repo is used in a lot of places! 🙂

Kingdon Barrett added 6 commits August 31, 2021 08:15
unrecognized import path "io/fs": import path does not begin with hostname

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
It looks like this can be safely removed

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
from within build container that failed, the error message said:

\# make
go get -tags netgo ./.
go: cannot find main module, but found .git/config in /go/src
	to create a module there, run:
	cd .. && go mod init
Makefile:6: recipe for target 'cover' failed
make: *** [cover] Error 1

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
Signed-off-by: Kingdon Barrett <kingdon@weave.works>
add 'go get -tags netgo \

                github.com/FiloSottile/gvt \
                github.com/client9/misspell/cmd/misspell \
                github.com/fzipp/gocyclo \
                github.com/gogo/protobuf/gogoproto \
                github.com/gogo/protobuf/protoc-gen-gogoslick \
                golang.org/x/lint/golint \
                github.com/golang/protobuf/protoc-gen-go \
                github.com/kisielk/errcheck \
                github.com/mjibson/esc'
from build/golang/Dockerfile

(next commit probably ought to remove it from in there, where it is
likely wasted repeating the work)

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
$ docker run --rm -v "$PWD:/go/src" -w "/go/src" --entrypoint sh weaveworks
/build-golang -c "./lint"
go: downloading golang.org/x/tools v0.1.5
runner/runner.go:17:2: no required module provides package github.com/mgutz/ansi; to add it:
	go get github.com/mgutz/ansi
runner/runner.go:18:2: no required module provides package github.com/weaveworks/common/mflag; to add it:
	go get github.com/weaveworks/common/mflag
socks/main.go:12:2: no required module provides package github.com/armon/go-socks5; to add it:
	go get github.com/armon/go-socks5
socks/main.go:13:2: no required module provides package github.com/weaveworks/common/mflag; to add it:
	go get github.com/weaveworks/common/mflag
socks/main.go:14:2: no required module provides package github.com/weaveworks/common/mflagext; to add it:
	go get github.com/weaveworks/common/mflagext

Signed-off-by: Kingdon Barrett <kingdon@weave.works>
@kingdonb
Copy link

9cc4ff7

In this commit I added a go.mod file, I think this means the go get statements in build/golang/Dockerfile are redundant now. I have not transitioned apps myself from gopath to go mod before, I don't know if it makes sense to merge this as-is just because the tests are passing.

I think we need someone to take a quick look who knows about go mod migration (but at least I got the build working again)

Copy link
Contributor

@yiannistri yiannistri left a comment

Choose a reason for hiding this comment

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

LGTM 👍 At some point (in another PR) we should also get rid of gvt since it has being obsoleted by go modules.

@dholbach dholbach mentioned this pull request Aug 31, 2021
@kingdonb
Copy link

Thanks. If someone who knows anything about the release cycle of this package is comfortable merging this, I definitely won't stop you.

The build is green at least, if there are issues to follow up they can be handled in a separate PR. @morancj Back in your court?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants