Currently, we have a bunch of places in our code where a linter like
golangci-lint could help us out figuring out basic problems.
Given that golangci-lint would probably scream at us with tons and tons of
things to fix right away, maybe we could start with something smaller like go vet which is already capable of finding out about interesting things.
In the past, when we had golangci-lint, we had the lint job in the main
pipeline, not blocking anything (serving just as an indicator), but that didn't
motivate us at all when it comes to fixing things.
Starting small (with just go vet), we could probably make it block everything
as there's not really all that much to fix:
# github.com/concourse/concourse/atc
atc/user.go:8:2: struct field tag `json:"sub", omitempty` not compatible with reflect.StructTag.Get: key:"value" pairs not separated by spaces
# github.com/concourse/concourse/atc/metric
atc/metric/emit.go:103:2: self-assignment of emitter to emitter
# github.com/concourse/concourse/atc/metric/emitter
atc/metric/emitter/newrelic.go:226:8: using resp before checking for errors
# github.com/concourse/concourse/atc/gc
atc/gc/build_log_retention_calculator.go:55:10: github.com/concourse/concourse/atc.BuildLogRetention composite literal uses unkeyed fields
# github.com/concourse/concourse/atc/exec_test
atc/exec/artifact_input_step_test.go:44:35: github.com/concourse/concourse/atc.ArtifactInputPlan composite literal uses unkeyed fields
atc/exec/artifact_output_step_test.go:57:36: github.com/concourse/concourse/atc.ArtifactOutputPlan composite literal uses unkeyed fields
atc/exec/get_step_test.go:284:30: github.com/concourse/concourse/atc/runtime.GetArtifact composite literal uses unkeyed fields
atc/exec/put_step_test.go:309:54: github.com/concourse/concourse/atc.MetadataField composite literal uses unkeyed fields
# github.com/concourse/concourse/atc/worker/image_test
atc/worker/image/image_resource_fetcher_test.go:332:49: github.com/concourse/concourse/atc/worker.ImageFetcherSpec composite literal uses unkeyed fields
atc/worker/image/image_resource_fetcher_test.go:557:47: github.com/concourse/concourse/atc/worker.ImageFetcherSpec composite literal uses unkeyed fields
# github.com/concourse/concourse/go-concourse/concourse/internal_test
go-concourse/concourse/internal/connection_test.go:421:59: github.com/google/jsonapi.ErrorsPayload composite literal uses unkeyed fields
# github.com/concourse/concourse/atc/worker_test
atc/worker/client_test.go:370:21: github.com/concourse/concourse/atc/runtime.VersionResult composite literal uses unkeyed fields
atc/worker/client_test.go:372:27: github.com/concourse/concourse/atc.MetadataField composite literal uses unkeyed fields
atc/worker/fetch_source_test.go:151:21: github.com/concourse/concourse/atc/runtime.GetArtifact composite literal uses unkeyed fields
atc/worker/fetch_source_test.go:202:21: github.com/concourse/concourse/atc/runtime.GetArtifact composite literal uses unkeyed fields
atc/worker/fetch_source_test.go:222:39: github.com/concourse/concourse/atc.MetadataField composite literal uses unkeyed fields
atc/worker/worker_test.go:1514:29: code.cloudfoundry.org/garden.ContainerNotFoundError composite literal uses unkeyed fields
atc/worker/client_test.go:246:9: the cancel function returned by context.WithCancel should be called, not discarded, to avoid a context leak
# github.com/concourse/concourse/worker/backend
worker/backend/backend.go:106:18: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak
worker/backend/backend.go:157:18: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak
worker/backend/backend.go:225:18: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak
worker/backend/backend.go:266:18: the cancel function returned by context.WithTimeout should be called, not discarded, to avoid a context leak
Also, to make sure we (any contributor actually) don't end up introducing more
problems, we could have the job added to the prs pipeline too.
thanks!
Currently, we have a bunch of places in our code where a linter like
golangci-lintcould help us out figuring out basic problems.Given that
golangci-lintwould probably scream at us with tons and tons ofthings to fix right away, maybe we could start with something smaller like
go vetwhich is already capable of finding out about interesting things.In the past, when we had
golangci-lint, we had the lint job in the mainpipeline, not blocking anything (serving just as an indicator), but that didn't
motivate us at all when it comes to fixing things.
Starting small (with just
go vet), we could probably make it block everythingas there's not really all that much to fix:
Also, to make sure we (any contributor actually) don't end up introducing more
problems, we could have the job added to the
prspipeline too.thanks!