From f69d8bcfac5eb54660904d11cbcb01de47860f40 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Mon, 28 May 2018 21:28:29 +0200 Subject: [PATCH 1/2] Replace gometalinter with GolangCI-Lint Running test.sh is down from 34s to 20s on my machine. Also, from now on, run go lint. Signed-off-by: David Gageot --- hack/gometalinter.json | 20 ------------------- hack/{gometalinter.sh => linter.sh} | 30 ++++++++++++++++------------- pkg/skaffold/kubernetes/log.go | 4 +--- pkg/skaffold/kubernetes/wait.go | 3 +-- test.sh | 2 +- 5 files changed, 20 insertions(+), 39 deletions(-) delete mode 100644 hack/gometalinter.json rename hack/{gometalinter.sh => linter.sh} (60%) diff --git a/hack/gometalinter.json b/hack/gometalinter.json deleted file mode 100644 index bfe78ec0916..00000000000 --- a/hack/gometalinter.json +++ /dev/null @@ -1,20 +0,0 @@ - -{ - "Vendor": true, - "EnableGC": true, - "Debug": false, - "Sort": ["linter", "severity", "path"], - "Enable": [ - "deadcode", - "gofmt", - "goimports", - "gosimple", - "ineffassign", - "interfacer", - "unconvert", - "vet", - "staticcheck" - ], - - "LineLength": 200 -} diff --git a/hack/gometalinter.sh b/hack/linter.sh similarity index 60% rename from hack/gometalinter.sh rename to hack/linter.sh index 8b3c85112aa..40b7175a045 100755 --- a/hack/gometalinter.sh +++ b/hack/linter.sh @@ -16,18 +16,22 @@ set -e -o pipefail -SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" - -install_gometalinter() { - echo "Installing gometalinter.v2" - go get -u gopkg.in/alecthomas/gometalinter.v2 - gometalinter.v2 --install -} - -if ! [ -x "$(command -v gometalinter.v2)" ]; then - install_gometalinter +if ! [ -x "$(command -v golangci-lint)" ]; then + echo "Installing GolangCI-Lint" + go get -u github.com/golangci/golangci-lint/cmd/golangci-lint fi -gometalinter.v2 \ - --deadline 5m \ - --config $SCRIPTDIR/gometalinter.json ./... +golangci-lint run \ + --no-config \ + -E goimports \ + -E interfacer \ + -E unconvert \ + -E goconst \ + -E maligned \ + -D errcheck + +# From now on, run go lint. +golangci-lint run \ + --disable-all \ + -E golint \ + --new-from-rev bed41e9a77431990cc8504c0955252c851934b89 \ No newline at end of file diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index dccdc3e379a..9d288a7b347 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -60,7 +60,6 @@ func NewLogAggregator(out io.Writer, podSelector PodSelector, colorPicker ColorP } func (a *LogAggregator) Start(ctx context.Context) error { - // Start logs kubeclient, err := Client() if err != nil { return errors.Wrap(err, "getting k8s client") @@ -101,8 +100,7 @@ func (a *LogAggregator) Start(ctx context.Context) error { return nil } -// nolint: interfacer -func (a *LogAggregator) streamLogs(ctx context.Context, client corev1.CoreV1Interface, pod *v1.Pod) error { +func (a *LogAggregator) streamLogs(ctx context.Context, client corev1.PodsGetter, pod *v1.Pod) error { pods := client.Pods(pod.Namespace) if err := WaitForPodReady(pods, pod.Name); err != nil { return errors.Wrap(err, "waiting for pod ready") diff --git a/pkg/skaffold/kubernetes/wait.go b/pkg/skaffold/kubernetes/wait.go index 145676b55ef..e1db0dfc857 100644 --- a/pkg/skaffold/kubernetes/wait.go +++ b/pkg/skaffold/kubernetes/wait.go @@ -116,8 +116,7 @@ func (s *PodStore) Stop() { close(s.stopCh) } -// nolint: interfacer -func NewPodStore(c kubernetes.Interface, namespace string, label labels.Selector, field fields.Selector) *PodStore { +func NewPodStore(c kubernetes.Interface, namespace string, label fmt.Stringer, field fmt.Stringer) *PodStore { lw := &cache.ListWatch{ ListFunc: func(options meta_v1.ListOptions) (runtime.Object, error) { options.LabelSelector = label.String() diff --git a/test.sh b/test.sh index da31177215c..d5ecab40998 100755 --- a/test.sh +++ b/test.sh @@ -31,7 +31,7 @@ echo "Running validation scripts..." scripts=( "hack/boilerplate.sh" "hack/gofmt.sh" - "hack/gometalinter.sh" + "hack/linter.sh" "hack/dep.sh" ) fail=0 From 9bddebf89422cf62a38ab2421dcba0f061a64cbf Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 31 May 2018 21:41:01 +0200 Subject: [PATCH 2/2] Fix lint errors Signed-off-by: David Gageot --- pkg/skaffold/build/local_test.go | 2 +- pkg/skaffold/build/tag/date_time.go | 5 +++-- pkg/skaffold/build/tag/date_time_test.go | 8 -------- pkg/skaffold/build/tag/env_template.go | 2 +- pkg/skaffold/docker/image_test.go | 2 -- pkg/skaffold/kubernetes/wait_test.go | 14 -------------- pkg/skaffold/runner/runner.go | 2 +- pkg/skaffold/util/env_template_test.go | 4 ++-- 8 files changed, 8 insertions(+), 31 deletions(-) diff --git a/pkg/skaffold/build/local_test.go b/pkg/skaffold/build/local_test.go index 8149e5c20ea..2263fb75ca9 100644 --- a/pkg/skaffold/build/local_test.go +++ b/pkg/skaffold/build/local_test.go @@ -69,9 +69,9 @@ func TestLocalRun(t *testing.T) { out io.Writer api docker.DockerAPIClient tagger tag.Tagger - localCluster bool artifacts []*v1alpha2.Artifact expected []Build + localCluster bool shouldErr bool }{ { diff --git a/pkg/skaffold/build/tag/date_time.go b/pkg/skaffold/build/tag/date_time.go index f41ad0dc58f..52519322662 100644 --- a/pkg/skaffold/build/tag/date_time.go +++ b/pkg/skaffold/build/tag/date_time.go @@ -31,12 +31,13 @@ type dateTimeTagger struct { timeFn func() time.Time } -func NewDateTimeTagger(format, timezone string) (*dateTimeTagger, error) { +// NewDateTimeTagger creates a tagger from a date format and timezone. +func NewDateTimeTagger(format, timezone string) Tagger { return &dateTimeTagger{ Format: format, TimeZone: timezone, timeFn: func() time.Time { return time.Now() }, - }, nil + } } // GenerateFullyQualifiedImageName tags an image with the supplied image name and the current timestamp diff --git a/pkg/skaffold/build/tag/date_time_test.go b/pkg/skaffold/build/tag/date_time_test.go index 3669088316e..9706c45b1f6 100644 --- a/pkg/skaffold/build/tag/date_time_test.go +++ b/pkg/skaffold/build/tag/date_time_test.go @@ -23,14 +23,6 @@ import ( "github.com/GoogleContainerTools/skaffold/testutil" ) -type mockClock struct { - time time.Time -} - -func (m mockClock) Now() time.Time { - return m.time -} - func TestDateTime_GenerateFullyQualifiedImageName(t *testing.T) { aLocalTimeStamp := time.Date(2015, 03, 07, 11, 06, 39, 123456789, time.Local) localZone, _ := aLocalTimeStamp.Zone() diff --git a/pkg/skaffold/build/tag/env_template.go b/pkg/skaffold/build/tag/env_template.go index b1cf3d94fd1..60013d1068e 100644 --- a/pkg/skaffold/build/tag/env_template.go +++ b/pkg/skaffold/build/tag/env_template.go @@ -30,7 +30,7 @@ type envTemplateTagger struct { } // NewEnvTemplateTagger creates a new envTemplateTagger -func NewEnvTemplateTagger(t string) (*envTemplateTagger, error) { +func NewEnvTemplateTagger(t string) (Tagger, error) { tmpl, err := util.ParseEnvTemplate(t) if err != nil { return nil, errors.Wrap(err, "parsing template") diff --git a/pkg/skaffold/docker/image_test.go b/pkg/skaffold/docker/image_test.go index 217625d4522..ddf96b8e636 100644 --- a/pkg/skaffold/docker/image_test.go +++ b/pkg/skaffold/docker/image_test.go @@ -37,7 +37,6 @@ func TestMain(m *testing.M) { type testImageAPI struct { description string imageName string - imageID string tagToImageID map[string]string shouldErr bool expected string @@ -96,7 +95,6 @@ func TestRunBuild(t *testing.T) { { description: "build", tagToImageID: map[string]string{}, - imageID: "sha256:test", expected: "test", }, { diff --git a/pkg/skaffold/kubernetes/wait_test.go b/pkg/skaffold/kubernetes/wait_test.go index 91ed6c8a785..00373a1259f 100644 --- a/pkg/skaffold/kubernetes/wait_test.go +++ b/pkg/skaffold/kubernetes/wait_test.go @@ -79,20 +79,6 @@ var podBadPhase = &v1.Pod{ }, } -var podDifferentName = &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pod_different_name", - }, - Status: v1.PodStatus{ - Conditions: []v1.PodCondition{ - { - Type: v1.PodScheduled, - }, - }, - Phase: "not a real phase", - }, -} - func TestWaitForPodReady(t *testing.T) { var tests = []struct { description string diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 5bf98e81e0c..398023b9571 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -141,7 +141,7 @@ func getTagger(t v1alpha2.TagPolicy, customTag string) (tag.Tagger, error) { return &tag.GitCommit{}, nil case t.DateTimeTagger != nil: - return tag.NewDateTimeTagger(t.DateTimeTagger.Format, t.DateTimeTagger.TimeZone) + return tag.NewDateTimeTagger(t.DateTimeTagger.Format, t.DateTimeTagger.TimeZone), nil default: return nil, fmt.Errorf("Unknown tagger for strategy %+v", t) diff --git a/pkg/skaffold/util/env_template_test.go b/pkg/skaffold/util/env_template_test.go index a0669ffe3c6..97ef3e58bc7 100644 --- a/pkg/skaffold/util/env_template_test.go +++ b/pkg/skaffold/util/env_template_test.go @@ -60,12 +60,12 @@ func TestEnvTemplate_ExecuteEnvTemplate(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - test_template := template.Must(template.New("").Parse(test.template)) + testTemplate := template.Must(template.New("").Parse(test.template)) OSEnviron = func() []string { return test.env } - got, err := ExecuteEnvTemplate(test_template, test.customMap) + got, err := ExecuteEnvTemplate(testTemplate, test.customMap) testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.want, got) }) }