-
Notifications
You must be signed in to change notification settings - Fork 612
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
[18.03 backport] Use gometalinter; switch from x/net/context -> context #2827
Closed
Commits on Feb 12, 2019
-
Adding a Dockerfile and making it easy to use it for dev
Credit goes to dperny (moby#2687) **- What I did** Adds a Dockerfile for the swarmkit project, to easily get off the ground. Modifies the Makefile to make intelligent use of Docker. Also made small clean up changes to the Makefile. **- How I did it** Modifies the Makefile to have two paths: containerized.mk, which builds the docker image and forwards any make targets to a container, and direct.mk, which encompasses the old Makefile's workflow. By default, nothing will run inside a container. Set the environment variable `DOCKER_SWARMKIT_USE_CONTAINER` to use dockerized making. Also leverages docker-sync for synchronizing code to the container if the `DOCKER_SWARMKIT_USE_DOCKER_SYNC` env variable is set; comes in handy on Macs, for example. **- How to test it** Set `DOCKER_SWARMKIT_USE_CONTAINER` and verify that your favorite make targets all work! Signed-off-by: Jean Rouge <jer329@cornell.edu> (cherry picked from commit f8c048c) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for 73b733b - Browse repository at this point
Copy the full SHA 73b733bView commit details -
Instead of running many small source code checkers and linters one by one, let's use gometalinter that runs them all in parallel. While at it, remove the individual make targets (fmt, vet, lint, ineffassign, and misspell) and replace with a single check target. One thing it provides is faster source validation. BEFORE: real 0m24.025s user 1m2.646s sys 0m3.860s (note these timings are without building binaries, which for some reason was a dependency of the vet target) AFTER: real 0m6.330s user 0m20.255s sys 0m1.019s In addition to this, it is now way easier to add/remove the checks, as well as to filter out some errors from linters that we consider false positives. [v2: add 2m deadline] [v3: use gometalinter --install; move configuration to .json] Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 318574d) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for 17a74c3 - Browse repository at this point
Copy the full SHA 17a74c3View commit details -
Switch from x/net/context -> context
1. Since Go 1.7, context is a standard package. Since Go 1.9, everything that is provided by "x/net/context" is a couple of type aliases to types in "context". 2. While at it, fix the order of imports (done by goimports and some shell trickery, see below). 3. Also, when the standard context package is used, the errors of not calling cancel() are detected/reported by go vet: > the cancel function returned by context.WithTimeout should be called, > not discarded, to avoid a context leak (vet) This essentially asks to call cancel() as otherwise a stray timer is leaked. Fix a few such issues, mostly in _test.go files. 4. Since in func (*session).start (see agent/session.go) we deliberately do not want to cancel the context, govet gives us a couple of errors: > agent/session.go:140: the cancelSession function is not used > on all paths (possible context leak) > agent/session.go:163: this return statement may be reached > without using the cancelSession var defined on line 140 To silence these, use `// nolint: vet` mark in a couple of places (this is a feature of gometalinter). Oh, the conversion (items 1 and 2 above) was performed by this shell script: ```sh FILES=$* test -z "$FILES" && FILES=$(git ls-files \*.go | grep -v ^vendor/ | grep -v .pb.go$) for f in $FILES; do sed -i 's|"golang.org/x/net/context"|"context"|' $f goimports -w $f for i in 1 2; do awk '/^$/ {e=1; next;} /\t"context"$/ {e=0;} {if (e) {print ""; e=0}; print;}' < $f > $f.new && mv $f.new $f goimports -w $f done echo -n . done echo ``` Multiple `goimports` calls and some awk trickery is needed to iron out incorrect formatting (excessive empty lines) from the import statements. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit b25f50f) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for 3a06e5d - Browse repository at this point
Copy the full SHA 3a06e5dView commit details -
gometalinter: add deadcode linter
...and fix the following initial bunch of warnings: agent/session.go:18:1:warning: errSessionDisconnect is unused (deadcode) agent/errors.go:7:1:warning: errTaskNoController is unused (deadcode) agent/errors.go:7:1:warning: errTaskStatusUpdateNoChange is unused (deadcode) agent/errors.go:7:1:warning: errTaskNotAssigned is unused (deadcode) agent/errors.go:7:1:warning: errTaskInvalid is unused (deadcode) ca/transport.go:21:1:warning: timeoutError is unused (deadcode) ca/config.go:29:1:warning: nodeCSRFilename is unused (deadcode) cmd/swarmctl/node/common.go:58:1:warning: changeNodeMembership is unused (deadcode) integration/cluster.go:44:1:warning: newTestCluster is unused (deadcode) manager/orchestrator/global/global.go:590:1:warning: isTaskCompleted is unused (deadcode) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 04ae7e3) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for dac1440 - Browse repository at this point
Copy the full SHA dac1440View commit details -
gometalinter: add deadcode, goimports, unconvert
... and fix some warnings from unconvert: ca/config.go:627:58:warning: unnecessary conversion (unconvert) manager/allocator/cnmallocator/portallocator.go:410:38:warning: unnecessary conversion (unconvert) manager/allocator/cnmallocator/portallocator.go:415:50:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:200:47:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:210:45:warning: unnecessary conversion (unconvert) manager/controlapi/service.go:220:35:warning: unnecessary conversion (unconvert) manager/dispatcher/nodes.go:159:33:warning: unnecessary conversion (unconvert) manager/state/store/memory.go:692:91:warning: unnecessary conversion (unconvert) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 278edc2) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for f8340c8 - Browse repository at this point
Copy the full SHA f8340c8View commit details -
... and fix warnings reported by it: agent/exec/dockerapi/adapter.go:147:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) agent/testutils/fakes.go:143:2:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) agent/testutils/fakes.go:151:2:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) ca/certificates_test.go:708:2:warning: should use for range instead of for { select {} } (S1000) (gosimple) ca/config.go:630:12:warning: should use time.Until instead of t.Sub(time.Now()) (S1024) (gosimple) ca/config_test.go:790:3:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) ca/external_test.go:116:3:warning: should use a simple channel send/receive instead of select with a single case (S1000) (gosimple) cmd/swarm-bench/collector.go:26:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:51:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:89:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) cmd/swarmctl/node/common.go:172:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) ioutils/ioutils_test.go:28:5:warning: should use !bytes.Equal(actual, expected) instead (S1004) (gosimple) manager/allocator/cnmallocator/networkallocator.go:818:3:warning: should merge variable declaration with assignment on next line (S1021) (gosimple) manager/constraint/constraint.go:59:7:warning: should omit comparison to bool constant, can be simplified to !matched (S1002) (gosimple) manager/constraint/constraint.go:67:7:warning: should omit comparison to bool constant, can be simplified to !matched (S1002) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher.go:1095:5:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) manager/dispatcher/dispatcher_test.go:2090:2:warning: redundant return statement (S1023) (gosimple) manager/manager.go:1005:4:warning: should replace loop with m.config.NetworkConfig.DefaultAddrPool = append(m.config.NetworkConfig.DefaultAddrPool, cluster.DefaultAddressPool...) (S1011) (gosimple) manager/metrics/collector.go:191:2:warning: redundant return statement (S1023) (gosimple) manager/metrics/collector.go:222:2:warning: redundant return statement (S1023) (gosimple) manager/orchestrator/replicated/update_test.go:53:3:warning: should use for range instead of for { select {} } (S1000) (gosimple) manager/orchestrator/taskinit/init.go:83:32:warning: should use time.Until instead of t.Sub(time.Now()) (S1024) (gosimple) manager/state/raft/raft.go:1185:2:warning: should use 'return <expr>' instead of 'if <expr> { return <bool> }; return <bool>' (S1008) (gosimple) manager/state/raft/raft.go:1594:2:warning: 'if err != nil { return err }; return nil' can be simplified to 'return err' (S1013) (gosimple) node/node.go:1209:2:warning: redundant return statement (S1023) (gosimple) node/node.go:1219:2:warning: redundant return statement (S1023) (gosimple) watch/sinks_test.go:42:2:warning: should merge variable declaration with assignment on next line (S1021) (gosimple) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com> (cherry picked from commit 0e8bb70) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for 51a6be3 - Browse repository at this point
Copy the full SHA 51a6be3View commit details -
gometalinter dropped support for gosimple, which is deprecated anyway and has been subsumed by staticcheck. This commit removes gosimple from our list of enabled linters (as it's no longer valid). It does not enable staticcheck, because staticcheck throws too many errors. Signed-off-by: Drew Erny <drew.erny@docker.com> (cherry picked from commit 3bfc201) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Configuration menu - View commit details
-
Copy full SHA for 77a5597 - Browse repository at this point
Copy the full SHA 77a5597View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.