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.

Sign up for GitHub

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

Jump to bottom

*: fix goimports linter #16628

Merged
merged 2 commits into from
Sep 22, 2023
Merged

*: fix goimports linter #16628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa97484
*: enable goimports in verify-lint
fuweid Sep 21, 2023
46df6ab
*: remove goimports(_fix)_pass and add lint_fix_pass
fuweid Sep 21, 2023
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
14 changes: 3 additions & 11 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ fuzz:

verify: verify-gofmt verify-bom verify-lint verify-dep verify-shellcheck verify-goword \
verify-govet verify-license-header verify-receiver-name verify-mod-tidy verify-shellcheck \
verify-shellws verify-proto-annotations verify-genproto verify-goimport verify-yamllint \
verify-shellws verify-proto-annotations verify-genproto verify-yamllint \
verify-govet-shadow
fix: fix-goimports fix-bom fix-lint fix-yamllint
fix: fix-bom fix-lint fix-yamllint
Copy link
Member

Choose a reason for hiding this comment

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

It seems that you added lint_fix_pass to fix goimport issue, but I do not see where to verify the goimport issue after removing the verify-goimport? Did I miss anything?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was surprised when I prepared this patch. The problem is that the old goimports_pass was using run go list --f "{{with \$d:=.}}{{range .TestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./... to list test code. It can only list out the test files using the package name as same as the folder name. For example, the backend package has several test files but the go list only shows two files.

➜  backend git:(main) pwd
/home/fuwei/go/src/go.etcd.io/etcd/server/storage/backend

➜  backend git:(main) ls -al *_test.go
-rw-r--r-- 1 fuwei fuwei 1440 Sep 22 00:04 backend_bench_test.go
-rw-r--r-- 1 fuwei fuwei 8741 Sep 22 00:04 backend_test.go
-rw-r--r-- 1 fuwei fuwei 4838 Sep 21 20:16 batch_tx_test.go
-rw-r--r-- 1 fuwei fuwei  845 Sep  9 21:55 export_test.go
-rw-r--r-- 1 fuwei fuwei 3696 Sep 22 00:04 hooks_test.go
-rw-r--r-- 1 fuwei fuwei 2275 Sep 17 10:54 tx_buffer_test.go
-rw-r--r-- 1 fuwei fuwei 3010 Sep 18 17:10 verify_test.go

➜  backend git:(main) go list  --f "{{with \$d:=.}}{{range .TestGoFiles}}{{\$d.Dir}}/{{.}}{{\"\n\"}}{{end}}{{end}}" ./...
/home/fuwei/go/src/go.etcd.io/etcd/server/storage/backend/export_test.go
/home/fuwei/go/src/go.etcd.io/etcd/server/storage/backend/tx_buffer_test.go

There are several test files using backend_test instead of backend. So, after switch to golangci-lint, we can see several errors as the following result described.

'lint' started at Fri Sep 22 00:01:30 HKT 2023
% (cd api && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd pkg && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd client/pkg && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd client/internal/v2 && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd client/v3 && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd server && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd etcdutl && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd etcdctl && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% (cd tests && 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...')
% 'golangci-lint' 'run' '--config' '/home/fuwei/go/src/go.etcd.io/etcd/tools/.golangci.yaml' '--fix' './...'
'lint' PASSED and completed at Fri Sep 22 00:01:33 HKT 2023
SUCCESS
➜  etcd git:(main) ✗ gst
On branch main
Your branch is up to date with 'upstream/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        modified:   scripts/test.sh
        modified:   server/storage/backend/backend_bench_test.go
        modified:   server/storage/backend/backend_test.go
        modified:   server/storage/backend/hooks_test.go
        modified:   server/storage/datadir/datadir_test.go
        modified:   tests/integration/clientv3/concurrency/session_test.go
        modified:   tests/integration/clientv3/connectivity/black_hole_test.go
        modified:   tests/integration/clientv3/connectivity/dial_test.go
        modified:   tests/integration/clientv3/connectivity/network_partition_test.go
        modified:   tests/integration/clientv3/lease/leasing_test.go
        modified:   tests/integration/clientv3/snapshot/v3_snapshot_test.go
        modified:   tests/integration/embed/embed_test.go
        modified:   tests/integration/revision_test.go
        modified:   tests/integration/snapshot/v3_snapshot_test.go
        modified:   tests/integration/v2store/store_tag_test.go
        modified:   tests/integration/v2store/store_test.go
        modified:   tools/.golangci.yaml

Most of them were modified manually. If it's unnecessary, I can rollback it.
I don't have knowledge about the X_test package naming. Maybe we can change them to X in the future.

Copy link
Member

Choose a reason for hiding this comment

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

My question is... if there is any not well organised import entries, is the github workflow able to detect it in future?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/etcd-io/etcd/pull/16628/files#diff-fe909f04d646d2ac68018e03d59a12d2b2ed8201f3618a9547c35d27ab255e25

Added the goimports in the golangci-linter. The CI can verify it.

The linter can detect the issue that both the third-party pkgs and go.etcd.io pkgs are in the same group.
The case is descripted by the following code.

imports (
     "testing"

     "third-party/pkg/xxxx"
     "go.etcd.io/etcd/xxx"
)

But it can't detect the issue that there is multiple go.etcd.io groups. That's why I have to modify the groups manually.

imports (
     "testing"

     "third-party/pkg/xxxx"

      "go.etcd.io/etcd/xxx"

      "go.etcd.io/etcd/xxy"

      "third-party/pkg/xxxy"
)

Not sure it answers your question.

./scripts/fix.sh

.PHONY: verify-gofmt
Expand All @@ -91,7 +91,7 @@ verify-lint:

.PHONY: fix-lint
fix-lint:
golangci-lint run --config tools/.golangci.yaml --fix
PASSES="lint_fix" ./scripts/test.sh

.PHONY: verify-shellcheck
verify-shellcheck:
Expand Down Expand Up @@ -129,14 +129,6 @@ verify-proto-annotations:
verify-genproto:
PASSES="genproto" ./scripts/test.sh

.PHONY: verify-goimport
verify-goimport:
PASSES="goimport" ./scripts/test.sh

.PHONY: fix-goimports
fix-goimports:
./scripts/fix-goimports.sh

.PHONY: verify-yamllint
verify-yamllint:
yamllint --config-file tools/.yamllint .
Expand Down
4 changes: 2 additions & 2 deletions client/pkg/logutil/zap_journal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ import (
"os"
"path/filepath"

"go.etcd.io/etcd/client/pkg/v3/systemd"

"github.com/coreos/go-systemd/v22/journal"
"go.uber.org/zap/zapcore"

"go.etcd.io/etcd/client/pkg/v3/systemd"
)

// NewJournalWriter wraps "io.Writer" to redirect log output
Expand Down
4 changes: 2 additions & 2 deletions client/pkg/transport/listener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ import (
"strings"
"time"

"go.uber.org/zap"

"go.etcd.io/etcd/client/pkg/v3/fileutil"
"go.etcd.io/etcd/client/pkg/v3/tlsutil"
"go.etcd.io/etcd/client/pkg/v3/verify"

"go.uber.org/zap"
)

// NewListener creates a new listner.
Expand Down
4 changes: 1 addition & 3 deletions client/v3/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ import (
"github.com/coreos/go-semver/semver"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.uber.org/zap"
"go.uber.org/zap/zaptest"
"google.golang.org/grpc"

"go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/api/v3/version"
"go.etcd.io/etcd/client/pkg/v3/testutil"

"google.golang.org/grpc"
)

func NewClient(t *testing.T, cfg Config) (*Client, error) {
Expand Down
4 changes: 2 additions & 2 deletions client/v3/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ package clientv3
import (
"context"

"google.golang.org/grpc"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/client/pkg/v3/types"

"google.golang.org/grpc"
)

type (
Expand Down
6 changes: 3 additions & 3 deletions client/v3/lease.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"sync"
"time"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"

"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
)

type (
Expand Down
6 changes: 3 additions & 3 deletions client/v3/leasing/kv.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ import (
"sync"
"time"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/mvccpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
v3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/concurrency"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

type leasingKV struct {
Expand Down
4 changes: 2 additions & 2 deletions client/v3/mock/mockserver/mockserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (
"os"
"sync"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"

"google.golang.org/grpc"
"google.golang.org/grpc/resolver"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
)

// MockServer provides a mocked out grpc server of the etcdserver interface.
Expand Down
6 changes: 3 additions & 3 deletions client/v3/naming/endpoints/endpoints_impl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ import (
"errors"
"strings"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/naming/endpoints/internal"

"go.uber.org/zap"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/naming/endpoints/internal"
)

type endpointManager struct {
Expand Down
6 changes: 3 additions & 3 deletions client/v3/naming/resolver/resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ import (
"strings"
"sync"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/naming/endpoints"

"google.golang.org/grpc/codes"
gresolver "google.golang.org/grpc/resolver"
"google.golang.org/grpc/status"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/naming/endpoints"
)

type builder struct {
Expand Down
6 changes: 3 additions & 3 deletions client/v3/retry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ package clientv3
import (
"context"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
)

type retryPolicy uint8
Expand Down
4 changes: 2 additions & 2 deletions client/v3/txn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import (
"context"
"sync"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"

"google.golang.org/grpc"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
)

// Txn is the interface that wraps mini-transactions.
Expand Down
8 changes: 4 additions & 4 deletions etcdctl/ctlv3/command/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import (
"sync"
"time"

v3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.etcd.io/etcd/pkg/v3/report"

"github.com/cheggaaa/pb/v3"
"github.com/spf13/cobra"
"golang.org/x/time/rate"

v3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.etcd.io/etcd/pkg/v3/report"
)

var (
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/elect_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import (
"os/signal"
"syscall"

"github.com/spf13/cobra"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/concurrency"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

var (
Expand Down
6 changes: 3 additions & 3 deletions etcdctl/ctlv3/command/ep_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ import (
"sync"
"time"

"github.com/spf13/cobra"
"go.uber.org/zap"

"go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/client/pkg/v3/logutil"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.etcd.io/etcd/pkg/v3/flags"

"github.com/spf13/cobra"
"go.uber.org/zap"
)

var epClusterEndpoints bool
Expand Down
9 changes: 4 additions & 5 deletions etcdctl/ctlv3/command/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,17 @@ import (
"time"

"github.com/bgentry/speakeasy"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"go.uber.org/zap"
"google.golang.org/grpc/grpclog"

"go.etcd.io/etcd/client/pkg/v3/logutil"
"go.etcd.io/etcd/client/pkg/v3/srv"
"go.etcd.io/etcd/client/pkg/v3/transport"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"
"go.etcd.io/etcd/pkg/v3/flags"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"go.uber.org/zap"
"google.golang.org/grpc/grpclog"
)

// GlobalFlags are flags that defined globally
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/lease_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import (
"fmt"
"strconv"

"github.com/spf13/cobra"

v3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

// NewLeaseCommand returns the cobra command for "lease".
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/lock_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ import (
"os/signal"
"syscall"

"github.com/spf13/cobra"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/concurrency"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

var lockTTL = 10
Expand Down
6 changes: 2 additions & 4 deletions etcdctl/ctlv3/command/make_mirror_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,13 @@ import (
"time"

"github.com/bgentry/speakeasy"

"go.etcd.io/etcd/pkg/v3/cobrautl"
"github.com/spf13/cobra"

"go.etcd.io/etcd/api/v3/mvccpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/client/v3/mirror"

"github.com/spf13/cobra"
"go.etcd.io/etcd/pkg/v3/cobrautl"
)

const (
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/printer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import (
"fmt"
"strings"

"github.com/dustin/go-humanize"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
v3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/dustin/go-humanize"
)

type printer interface {
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/printer_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ package command
import (
"os"

v3 "go.etcd.io/etcd/client/v3"

"github.com/olekukonko/tablewriter"

v3 "go.etcd.io/etcd/client/v3"
)

type tablePrinter struct{ printer }
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/txn_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ import (
"strconv"
"strings"

"github.com/spf13/cobra"

pb "go.etcd.io/etcd/api/v3/etcdserverpb"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

var txnInteractive bool
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import (
"strings"
"time"

"github.com/spf13/cobra"

pb "go.etcd.io/etcd/api/v3/mvccpb"
clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

func printKV(isHex bool, valueOnly bool, kv *pb.KeyValue) {
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/version_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ package command
import (
"fmt"

"go.etcd.io/etcd/api/v3/version"

"github.com/spf13/cobra"

"go.etcd.io/etcd/api/v3/version"
)

// NewVersionCommand prints out the version of etcd.
Expand Down
4 changes: 2 additions & 2 deletions etcdctl/ctlv3/command/watch_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import (
"os/exec"
"strings"

"github.com/spf13/cobra"

clientv3 "go.etcd.io/etcd/client/v3"
"go.etcd.io/etcd/pkg/v3/cobrautl"

"github.com/spf13/cobra"
)

var (
Expand Down
Loading
Loading