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

Replace gometalinter with GolangCI-Lint #619

Merged
merged 2 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 0 additions & 20 deletions hack/gometalinter.json

This file was deleted.

30 changes: 17 additions & 13 deletions hack/gometalinter.sh → hack/linter.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
{
Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/tag/date_time.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions pkg/skaffold/build/tag/date_time_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/tag/env_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 0 additions & 2 deletions pkg/skaffold/docker/image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -96,7 +95,6 @@ func TestRunBuild(t *testing.T) {
{
description: "build",
tagToImageID: map[string]string{},
imageID: "sha256:test",
expected: "test",
},
{
Expand Down
4 changes: 1 addition & 3 deletions pkg/skaffold/kubernetes/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Having this as a corev1interface instead of podsgetter was intentional, even though it failed linting.

Switching it gave me an error downstream. I don't think it would be covered by tests. I'll have to test it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean downstream? e2e tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep - i think the error I was thinking of was actually in wait.go where I had the nolint: interfacer. This LGTM

pods := client.Pods(pod.Namespace)
if err := WaitForPodReady(pods, pod.Name); err != nil {
return errors.Wrap(err, "waiting for pod ready")
Expand Down
3 changes: 1 addition & 2 deletions pkg/skaffold/kubernetes/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 0 additions & 14 deletions pkg/skaffold/kubernetes/wait_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/util/env_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
2 changes: 1 addition & 1 deletion test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down