Replace gometalinter with Golangci lint [carry 1797] #2173

thaJeztah · 2019-10-28T23:33:24Z

carries #1797
closes #1797

based on top of / depends on compose/loader: fix TestIsAbs not testing all combinations #2174 compose/loader: fix TestIsAbs not testing all combinations
depends on Gometalinter: raise deadline to 3 minutes #2172 "Gometalinter: raise deadline to 3 minutes"
- in case we want to backport that change, and not the switch to gometalinter
depends on kubernetes/conversion_test: use test-builders package #2180 kubernetes/conversion_test: use test-builders package

thaJeztah · 2019-10-29T14:35:55Z

This should be good for review now; but still needs a rebase after #2174 is merged

ping @kolyshkin @silvin-lubecki @vdemeester @tiborvass PTAL

vdemeester

LGTM

thaJeztah · 2019-10-29T14:54:48Z

oh, needs a minor rebase after #2172 was merged; will do so in a bit

silvin-lubecki

LGTM

thaJeztah · 2019-10-30T07:50:00Z

Ha! Linting is failing because I added options to a test-utility that I didn't use in the end

cli/command/service/client_test.go:75:41: `newService` - `opts` always receives `nil` (unparam)
func newService(id string, name string, opts ...func(*swarm.Service)) swarm.Service {
                                        ^
cli/command/stack/kubernetes/conversion_test.go:168:15: `withMode` - `mode` always receives `"replicated"` (unparam)
func withMode(mode string, replicas uint64) func(*swarm.Service) {
              ^

I did that of course to show how well the linter works 😜
Let me remove that option as it's easy to add back if someone needs it (or perhaps I can add a test for it, and all would be well then)

…taticcheck) Signed-off-by: Silvin Lubecki <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

…checked (errcheck) ``` cli/command/formatter/container_test.go:315:17: Error return value of `ContainerWrite` is not checked (errcheck) ContainerWrite(context.context, containers) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…l (scopelint) ``` cli/config/config_test.go:590:11: Using the variable on range scope `tc` in function literal (scopelint) SetDir(tc.dir) ^ cli/config/config_test.go:591:19: Using the variable on range scope `tc` in function literal (scopelint) f, err := Path(tc.path...) ^ cli/config/config_test.go:592:23: Using the variable on range scope `tc` in function literal (scopelint) assert.Equal(t, f, tc.expected) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…n literal (scopelint) ``` service/logs/parse_logs_test.go:26:35: Using the variable on range scope `testcase` in function literal (scopelint) actual, err := ParseLogDetails(testcase.line) ^ service/logs/parse_logs_test.go:27:7: Using the variable on range scope `testcase` in function literal (scopelint) if testcase.err != nil { ^ service/logs/parse_logs_test.go:28:26: Using the variable on range scope `testcase` in function literal (scopelint) assert.Error(t, err, testcase.err.Error()) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…iteral (scopelint) ``` templates/templates_test.go:74:29: Using the variable on range scope `testCase` in function literal (scopelint) assert.Check(t, is.Equal(testCase.expected, b.String())) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

… `overrideService` (scopelint) ``` cli/compose/loader/merge.go:64:41: Using a reference for the variable on range scope `overrideService` (scopelint) if err := mergo.Merge(&baseService, &overrideService, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(specials)); err != nil { ^ cli/compose/loader/loader_test.go:1587:28: Using the variable on range scope `testcase` in function literal (scopelint) config, err := loadYAML(testcase.yaml) ^ cli/compose/loader/loader_test.go:1590:58: Using the variable on range scope `testcase` in function literal (scopelint) assert.Check(t, is.DeepEqual(config.Services[0].Init, testcase.init)) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

… literal (scopelint) ``` e2e/cli-plugins/flags_test.go:135:27: Using the variable on range scope `args` in function literal (scopelint) res := icmd.RunCmd(run(args...)) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…n literal (scopelint) ``` cli/command/context/create_test.go:270:31: Using the variable on range scope `c` in function literal (scopelint) Name: c.name, ^ cli/command/context/create_test.go:271:31: Using the variable on range scope `c` in function literal (scopelint) Description: c.description, ^ cli/command/context/create_test.go:272:31: Using the variable on range scope `c` in function literal (scopelint) DefaultStackOrchestrator: c.orchestrator, cli/command/context/create_test.go:346:31: Using the variable on range scope `c` in function literal (scopelint) Name: c.name, ^ cli/command/context/create_test.go:347:31: Using the variable on range scope `c` in function literal (scopelint) Description: c.description, ^ cli/command/context/create_test.go:348:31: Using the variable on range scope `c` in function literal (scopelint) DefaultStackOrchestrator: c.orchestrator, ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…nction literal (scopelint) ``` cli/command/trust/key_load_test.go:121:27: Using the variable on range scope `keyID` in function literal (scopelint) testLoadKeyFromPath(t, keyID, keyBytes) ^ cli/command/trust/key_load_test.go:176:32: Using the variable on range scope `keyBytes` in function literal (scopelint) testLoadKeyTooPermissive(t, keyBytes) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

… literal (scopelint) ``` cli/command/cli_test.go:157:15: Using the variable on range scope `testcase` in function literal (scopelint) pingFunc: testcase.pingFunc, ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

``` cli/config/config_test.go:465: unnecessary trailing newline (whitespace) } cli/compose/interpolation/interpolation.go:56: unnecessary leading newline (whitespace) switch value := value.(type) { cli/compose/interpolation/interpolation.go:94: unnecessary trailing newline (whitespace) } cli/command/image/build/context.go:348: unnecessary trailing newline (whitespace) } internal/licenseutils/client_test.go:98: unnecessary leading newline (whitespace) func (c *fakeLicensingClient) LoadLocalLicense(ctx context.Context, dclnt licensing.WrappedDockerClient) (*model.Subscription, error) { cli/registry/client/fetcher.go:211: unnecessary leading newline (whitespace) for _, endpoint := range endpoints { ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

``` internal/licenseutils/client_test.go:98: unnecessary leading newline (whitespace) func (c *fakeLicensingClient) LoadLocalLicense(ctx context.Context, dclnt licensing.WrappedDockerClient) (*model.Subscription, error) { ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Sebastiaan van Stijn <[email protected]>

``` cli/command/container/opts_test.go:68:2: declaration has 3 blank identifiers (dogsled) _, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " ")) ^ cli/command/container/opts_test.go:542:2: declaration has 3 blank identifiers (dogsled) _, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"}) ^ cli/command/container/opts_test.go:603:2: declaration has 3 blank identifiers (dogsled) _, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"}) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…ion literal (scopelint) ``` cli/compose/template/template_test.go:279:31: Using the variable on range scope `tc` in function literal (scopelint) actual := ExtractVariables(tc.dict, defaultPattern) ^ cli/compose/template/template_test.go:280:41: Using the variable on range scope `tc` in function literal (scopelint) assert.Check(t, is.DeepEqual(actual, tc.expected)) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…n literal (scopelint) ``` cli/manifest/store/store_test.go:97:29: Using the variable on range scope `testcase` in function literal (scopelint) actual, err := store.Get(testcase.listRef, testcase.manifestRef) ^ cli/manifest/store/store_test.go:98:7: Using the variable on range scope `testcase` in function literal (scopelint) if testcase.expectedErr != "" { ^ cli/manifest/store/store_test.go:99:26: Using the variable on range scope `testcase` in function literal (scopelint) assert.Error(t, err, testcase.expectedErr) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…pelint) ``` opts/network_test.go:74:35: Using the variable on range scope `tc` in function literal (scopelint) assert.NilError(t, network.Set(tc.value)) ^ opts/network_test.go:102:40: Using the variable on range scope `tc` in function literal (scopelint) assert.ErrorContains(t, network.Set(tc.value), tc.expectedError) ^ opts/opts_test.go:270:30: Using the variable on range scope `tc` in function literal (scopelint) val, err := ValidateLabel(tc.value) ^ opts/opts_test.go:271:7: Using the variable on range scope `tc` in function literal (scopelint) if tc.expectedErr != "" { ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…ange scope `obj` (scopelint) ``` cli/command/stack/kubernetes/watcher_test.go:44:20: Using a reference for the variable on range scope `obj` (scopelint) if err := o.Add(&obj); err != nil { ^ cli/command/stack/kubernetes/watcher_test.go:49:20: Using a reference for the variable on range scope `obj` (scopelint) if err := o.Add(&obj); err != nil { ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…ion literal (scopelint) ``` cli/command/container/create_test.go:120:20: Using the variable on range scope `c` in function literal (scopelint) defer func() { c.ResponseCounter++ }() ^ cli/command/container/create_test.go:121:12: Using the variable on range scope `c` in function literal (scopelint) switch c.ResponseCounter { ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

…n function literal (scopelint) ``` cli/command/stack/kubernetes/convert_test.go:199:35: Using the variable on range scope `c` in function literal (scopelint) conv, err := NewStackConverter(c.version) ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

``` cli/command/service/client_test.go:75:41: `newService` - `opts` always receives `nil` (unparam) func newService(id string, name string, opts ...func(*swarm.Service)) swarm.Service { ^ ``` Signed-off-by: Sebastiaan van Stijn <[email protected]>

Signed-off-by: Silvin Lubecki <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2019-10-31T18:56:02Z

rebased; all green again; I'll go ahead and merge

GordonTheTurtle added the status/0-triage label Oct 28, 2019

thaJeztah force-pushed the carry_golangci_lint branch from 4d98ed0 to 9ac1954 Compare October 28, 2019 23:44

thaJeztah mentioned this pull request Oct 29, 2019

compose/loader: fix TestIsAbs not testing all combinations #2174

Merged

thaJeztah force-pushed the carry_golangci_lint branch 2 times, most recently from 277dadd to 4efb4fe Compare October 29, 2019 09:53

thaJeztah added area/testing process/cherry-pick status/2-code-review and removed status/0-triage labels Oct 29, 2019

thaJeztah force-pushed the carry_golangci_lint branch 10 times, most recently from 43a089c to d220897 Compare October 29, 2019 14:05

thaJeztah marked this pull request as ready for review October 29, 2019 14:35

thaJeztah requested a review from silvin-lubecki as a code owner October 29, 2019 14:35

vdemeester approved these changes Oct 29, 2019

View reviewed changes

thaJeztah force-pushed the carry_golangci_lint branch from d220897 to 3aac14a Compare October 29, 2019 15:05

silvin-lubecki approved these changes Oct 29, 2019

View reviewed changes

thaJeztah force-pushed the carry_golangci_lint branch from 3aac14a to 6b0d31d Compare October 30, 2019 09:59

thaJeztah mentioned this pull request Oct 30, 2019

kubernetes/conversion_test: use test-builders package #2180

Merged

opts/throttledevice.go:51:5: SA4003: unsigned values are never < 0 (s…

2962971

…taticcheck) Signed-off-by: Silvin Lubecki <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah and others added 21 commits October 31, 2019 19:22

e2e: remove unnecessary trailing newline (whitespace)

dd4d216

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Remove now obsolete gometalinter and use golangci-lint instead

b7e06f2

Signed-off-by: Silvin Lubecki <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah force-pushed the carry_golangci_lint branch from 6b0d31d to b7e06f2 Compare October 31, 2019 18:22

thaJeztah merged commit 968ce1a into docker:master Oct 31, 2019

thaJeztah deleted the carry_golangci_lint branch October 31, 2019 18:56

thaJeztah mentioned this pull request Jan 6, 2020

[19.03 backport] Replace gometalinter with Golangci lint [carry 1797] #2239

Merged

thaJeztah added process/cherry-picked and removed process/cherry-pick labels Jan 6, 2020

thaJeztah added this to the next milestone Feb 5, 2020

thaJeztah mentioned this pull request Feb 18, 2025

golangci-lint: fix invalid nakedret config, disallow for any func length #5836

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace gometalinter with Golangci lint [carry 1797] #2173

Replace gometalinter with Golangci lint [carry 1797] #2173

Uh oh!

thaJeztah commented Oct 28, 2019 •

edited

Loading

Uh oh!

thaJeztah commented Oct 29, 2019

Uh oh!

vdemeester left a comment

Uh oh!

thaJeztah commented Oct 29, 2019

Uh oh!

silvin-lubecki left a comment

Uh oh!

thaJeztah commented Oct 30, 2019

Uh oh!

thaJeztah commented Oct 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Replace gometalinter with Golangci lint [carry 1797] #2173

Replace gometalinter with Golangci lint [carry 1797] #2173

Uh oh!

Conversation

thaJeztah commented Oct 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 29, 2019

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 29, 2019

Uh oh!

silvin-lubecki left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Oct 30, 2019

Uh oh!

thaJeztah commented Oct 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thaJeztah commented Oct 28, 2019 •

edited

Loading