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

Moves lint and format to make, managed by bingo #132

Merged
merged 2 commits into from
Mar 26, 2021
Merged

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Mar 25, 2021

This removes a few shell scripts and in the process updates linters to
more current versions. For example, we were using golangci from 2019. To
adjust, some invalid statements were removed.

This also adds goimport formatting, used by some of our other projects.

The main part of this change is using https://github.com/bwplotka/bingo
as the linters we used were go programs anyway. Bingo generates make
entries you can use to on-demang install tools without affecting your
go.mod and without shell scripts.

Fixes #131

@@ -0,0 +1,12 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: files in .bingo are generated, so you can ignore them

.golangci.yml Outdated
- initClause
- methodExprCall
- nilValReturn
- octalLiteral
- offBy1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed all these lines to get rid of redundant warnings in output

@@ -97,7 +97,7 @@ func (r *Runtime) SaveConfig(name, config string) (string, error) {
return "", fmt.Errorf("Unable to create directory %q: %v", configDir, err)
}
filename := name + ".yaml"
err := ioutil.WriteFile(filepath.Join(configDir, filename), []byte(config), 0644)
err := ioutil.WriteFile(filepath.Join(configDir, filename), []byte(config), 0600)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

new version of the sec linter is more strict. I went with the advice instead of ignoring it.

Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@codefromthecrypt codefromthecrypt force-pushed the bingo branch 2 times, most recently from 3a21b59 to f8365f3 Compare March 25, 2021 06:44
@codefromthecrypt codefromthecrypt changed the title Moves lint and format to make, managed by bingo Moves lint and format to make, managed by bingo; completes GH action migration Mar 25, 2021
@codefromthecrypt
Copy link
Contributor Author

@mathetake can you disable circleci? I finished the almost done migration here https://app.circleci.com/settings/project/github/tetratelabs/getenvoy


jobs:
check:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these two were from circleci

@codefromthecrypt
Copy link
Contributor Author

some kooky stuff in tests like this. I'll get to the bottom of it..

vet: in github.com/containerd/containerd/remotes, can't import facts for package "context": open $WORK/b002/vet.out: no such file or directory
open /tmp/go-build2538038221/b1114/vet.cfg: no such file or directory

@codefromthecrypt
Copy link
Contributor Author

I think I have to rollback the circleci -> GH action thing until we can kill ginkgo #130 as normal tests are passing, but then the whole thing crashes. I was hoping I wouldn't have to fix circleci, but seems the case.

I'll summarize the crashing thing in #100

@codefromthecrypt
Copy link
Contributor Author

UPDATE: there's a panic in #133, once that's sorted I'll rebase this, eliminating the circleci thing, which I've documented in #100

@yskopets
Copy link
Member

@codefromthecrypt

I think I have to rollback the circleci -> GH action thing until we can kill ginkgo #130 as normal tests are passing

This statement is simply not true:

  • here is a CI build (based on this PR) that runs all Go std tests and excludes Ginkgo-based ones - it is failing
  • here is a CI build (based on this PR) that runs all Ginkgo-based tests and non-problematic Go std tests - it is successful

This removes a few shell scripts and in the process updates linters to
more current versions. For example, we were using golangci from 2019. To
adjust, some invalid statements were removed.

The main part of this change is using https://github.com/bwplotka/bingo
as the linters we used were go programs anyway. Bingo generates make
entries you can use to on-demang install tools without affecting your
go.mod and without shell scripts.

Co-authored-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt changed the title Moves lint and format to make, managed by bingo; completes GH action migration Moves lint and format to make, managed by bingo Mar 26, 2021
@codefromthecrypt
Copy link
Contributor Author

rebased over the go 1.16 PR and removed the circleci related changes

@codefromthecrypt
Copy link
Contributor Author

failure seems to repeat. will look into it..

/home/circleci/.getenvoy/builds/standard/1.11.0/linux_glibc/bin/envoy: text file busy

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

opened #136 about the strange test dep on a hard-coded envoy version

@codefromthecrypt
Copy link
Contributor Author

FYI "Run unit tests on Linux" is stale.. this is a known issue when you have added GH actions then remove it on the same PR. It still shows up in the output.

Copy link
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

thx!

@codefromthecrypt codefromthecrypt merged commit a9457a0 into master Mar 26, 2021
@codefromthecrypt codefromthecrypt deleted the bingo branch March 26, 2021 03:10
@codefromthecrypt
Copy link
Contributor Author

thanks for helping @mathetake

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.

self-contain binaries needed for the build
3 participants