Skip to content

Commit

Permalink
Fix local imports, add appropriate linter
Browse files Browse the repository at this point in the history
1. Modify Makefile's format target to format imports in three groups:
 - standard library packages;
 - third-party packages;
 - local packages (i.e. ones with the `github.com/google/cadvisor` prefix).

   Note goimports does the same job as gofmt, plus imports sorting. It
   can't do what gofmt -s does, but it was not done before (although
   this is checked according to .golangci.yml), so let's leave it as is.

   Note we can't use $(pkgs) and $(cmd_pkgs) in Makefile's format target,
   like it was done before, as we have to skip some auto-generated files
   (*.pb.go), so use find | grep | xargs pipe instead.

2. Add goimports linter with proper configuration to check imports
   formatting in CI. Check that the linter complains.

3. Actually implements these changes by running "make format". Check
   that the linter no longer complains.

4. Fix presubmit target to make sure it checks the new formatting rules.
   Instead of modifying build/check_gofmt.sh, let's use golangci-lint
   which already does this check (and more). While at it, let's remove
   vet target, as lint (golangci-lint) already calls it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Oct 19, 2021
1 parent 7263f2a commit 94cbb3c
Show file tree
Hide file tree
Showing 64 changed files with 126 additions and 108 deletions.
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ run:
min-confidence: 0
gofmt:
simplify: true
goimports:
local-prefixes: github.com/google/cadvisor
linters:
disable-all: true
enable:
Expand All @@ -22,6 +24,7 @@ linters:
- typecheck
- golint
- gofmt
- goimports
issues:
max-issues-per-linter: 0
max-same-issues: 0
Expand Down
15 changes: 5 additions & 10 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,10 @@ tidy:

format:
@echo ">> formatting code"
@$(GO) fmt $(pkgs)
@cd cmd && $(GO) fmt $(cmd_pkgs)

vet:
@echo ">> vetting code"
@$(GO) vet $(pkgs)
@cd cmd && $(GO) vet $(cmd_pkgs)
@# Can't use $(pkgs) and $(cmd_pkgs) here as we need to skip
@# auto-generated files. Note goimports is a superset of gofmt.
@find -name '*.go' | grep -Ev '^vendor/|.pb.go$$' | \
xargs goimports -w -local github.com/google/cadvisor

build: assets
@echo ">> building binaries"
Expand All @@ -88,9 +85,7 @@ docker-%:
docker-build:
@docker run --rm -w /go/src/github.com/google/cadvisor -v ${PWD}:/go/src/github.com/google/cadvisor golang:1.17 make build

presubmit: vet
@echo ">> checking go formatting"
@./build/check_gofmt.sh
presubmit: lint
@echo ">> checking go mod tidy"
@./build/check_gotidy.sh
@echo ">> checking file boilerplate"
Expand Down
3 changes: 2 additions & 1 deletion accelerators/nvidia_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@
package accelerators

import (
"github.com/google/cadvisor/stats"
"io/ioutil"
"os"
"path/filepath"
"testing"

"github.com/google/cadvisor/stats"

"github.com/mindprince/gonvml"
"github.com/stretchr/testify/assert"
)
Expand Down
27 changes: 0 additions & 27 deletions build/check_gofmt.sh

This file was deleted.

2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"path"
"strings"

"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"

"k8s.io/klog/v2"
)
Expand Down
2 changes: 1 addition & 1 deletion client/v2/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"strings"

v1 "github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/info/v2"
v2 "github.com/google/cadvisor/info/v2"
)

// Client represents the base URL for a cAdvisor client.
Expand Down
5 changes: 3 additions & 2 deletions client/v2/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,10 @@ import (
"testing"
"time"

"github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/info/v2"
"github.com/stretchr/testify/assert"

v1 "github.com/google/cadvisor/info/v1"
v2 "github.com/google/cadvisor/info/v2"
)

func cadvisorTestClient(path string, expectedPostObj *v1.ContainerInfoRequest, replyObj interface{}, t *testing.T) (*Client, *httptest.Server, error) {
Expand Down
3 changes: 2 additions & 1 deletion cmd/cadvisor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import (
"flag"
"testing"

"github.com/google/cadvisor/container"
"github.com/stretchr/testify/assert"

"github.com/google/cadvisor/container"
)

func TestTcpMetricsAreDisabledByDefault(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions cmd/internal/container/mesos/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ package mesos

import (
"fmt"
"net/url"
"sync"

"github.com/Rican7/retry"
"github.com/Rican7/retry/strategy"
mesos "github.com/mesos/mesos-go/api/v1/lib"
Expand All @@ -24,8 +27,6 @@ import (
mclient "github.com/mesos/mesos-go/api/v1/lib/client"
"github.com/mesos/mesos-go/api/v1/lib/encoding/codecs"
"github.com/mesos/mesos-go/api/v1/lib/httpcli"
"net/url"
"sync"
)

const (
Expand Down
3 changes: 2 additions & 1 deletion cmd/internal/container/mesos/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import (
"strings"
"time"

"k8s.io/klog/v2"

"github.com/google/cadvisor/container"
"github.com/google/cadvisor/container/libcontainer"
"github.com/google/cadvisor/fs"
info "github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/watcher"
"k8s.io/klog/v2"
)

var MesosAgentAddress = flag.String("mesos_agent", "127.0.0.1:5051", "Mesos agent address")
Expand Down
6 changes: 4 additions & 2 deletions cmd/internal/container/mesos/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
package mesos

import (
containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
"testing"

mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/stretchr/testify/assert"
"testing"

containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
)

func TestIsContainerName(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions cmd/internal/container/mesos/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ import (
"fmt"
"testing"

mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/stretchr/testify/assert"

"github.com/google/cadvisor/container"
containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
"github.com/google/cadvisor/fs"
info "github.com/google/cadvisor/info/v1"
mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/stretchr/testify/assert"
)

func PopulateContainer() *mContainer {
Expand Down
3 changes: 2 additions & 1 deletion cmd/internal/container/mesos/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
package install

import (
"k8s.io/klog/v2"

"github.com/google/cadvisor/cmd/internal/container/mesos"
"github.com/google/cadvisor/container"
"k8s.io/klog/v2"
)

func init() {
Expand Down
1 change: 1 addition & 0 deletions cmd/internal/container/mesos/mesos_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package mesos

import (
"fmt"

mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/mesos/mesos-go/api/v1/lib/agent"
)
Expand Down
3 changes: 2 additions & 1 deletion cmd/internal/container/mesos/mesos_agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ package mesos

import (
"fmt"
"testing"

mesos "github.com/mesos/mesos-go/api/v1/lib"
"github.com/mesos/mesos-go/api/v1/lib/agent"
"github.com/stretchr/testify/assert"
"testing"
)

func PopulateFrameworks(fwID string) *agent.Response_GetFrameworks {
Expand Down
1 change: 1 addition & 0 deletions cmd/internal/storage/bigquery/client/example/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"time"

"github.com/SeanDolphin/bqschema"

"github.com/google/cadvisor/cmd/internal/storage/bigquery/client"
)

Expand Down
2 changes: 1 addition & 1 deletion collector/collector_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"strings"
"time"

"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"
)

const metricLabelPrefix = "io.cadvisor.metric."
Expand Down
4 changes: 2 additions & 2 deletions collector/collector_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (
"testing"
"time"

"github.com/google/cadvisor/info/v1"

"github.com/stretchr/testify/assert"

v1 "github.com/google/cadvisor/info/v1"
)

type fakeCollector struct {
Expand Down
3 changes: 2 additions & 1 deletion collector/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import (
"time"

"encoding/json"
"github.com/google/cadvisor/info/v1"

v1 "github.com/google/cadvisor/info/v1"
)

type Config struct {
Expand Down
2 changes: 1 addition & 1 deletion collector/fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package collector
import (
"time"

"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"
)

type FakeCollectorManager struct {
Expand Down
2 changes: 1 addition & 1 deletion collector/generic_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"time"

"github.com/google/cadvisor/container"
"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"
)

type GenericCollector struct {
Expand Down
4 changes: 2 additions & 2 deletions collector/generic_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"os"
"testing"

"github.com/google/cadvisor/info/v1"
"github.com/stretchr/testify/assert"

containertest "github.com/google/cadvisor/container/testing"
"github.com/stretchr/testify/assert"
v1 "github.com/google/cadvisor/info/v1"
)

func TestEmptyConfig(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion collector/prometheus_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"github.com/prometheus/common/model"

"github.com/google/cadvisor/container"
"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"
)

type PrometheusCollector struct {
Expand Down
6 changes: 3 additions & 3 deletions collector/prometheus_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
"net/http/httptest"
"testing"

"github.com/google/cadvisor/info/v1"

containertest "github.com/google/cadvisor/container/testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

containertest "github.com/google/cadvisor/container/testing"
v1 "github.com/google/cadvisor/info/v1"
)

func TestPrometheus(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion collector/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ package collector
import (
"time"

"github.com/google/cadvisor/info/v1"
v1 "github.com/google/cadvisor/info/v1"
)

// TODO(vmarmol): Export to a custom metrics type when that is available.
Expand Down
7 changes: 4 additions & 3 deletions container/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ import (
"strings"
"time"

"github.com/google/cadvisor/container"
info "github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/utils"
"github.com/karrick/godirwalk"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/pkg/errors"
"golang.org/x/sys/unix"

"github.com/google/cadvisor/container"
info "github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/utils"

"k8s.io/klog/v2"
)

Expand Down
3 changes: 2 additions & 1 deletion container/common/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"

info "github.com/google/cadvisor/info/v1"
v2 "github.com/google/cadvisor/info/v2"
"github.com/stretchr/testify/assert"
)

func BenchmarkListDirectories(b *testing.B) {
Expand Down
3 changes: 2 additions & 1 deletion container/containerd/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ import (
"github.com/containerd/containerd/errdefs"
"golang.org/x/net/context"

specs "github.com/opencontainers/runtime-spec/specs-go"

"github.com/google/cadvisor/container"
"github.com/google/cadvisor/container/common"
containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
"github.com/google/cadvisor/fs"
info "github.com/google/cadvisor/info/v1"
specs "github.com/opencontainers/runtime-spec/specs-go"
)

type containerdContainerHandler struct {
Expand Down
5 changes: 3 additions & 2 deletions container/containerd/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ import (

"github.com/containerd/containerd/containers"
"github.com/containerd/typeurl"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"

"github.com/google/cadvisor/container"
containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
"github.com/google/cadvisor/fs"
info "github.com/google/cadvisor/info/v1"
specs "github.com/opencontainers/runtime-spec/specs-go"
"github.com/stretchr/testify/assert"
)

func init() {
Expand Down
Loading

0 comments on commit 94cbb3c

Please sign in to comment.