*: fix govet-shadow lint #16608
*: fix govet-shadow lint #16608
Conversation
fuweid
force-pushed
the
fix-shadow-vet
branch
from
7d8983f
to
b703656
Compare
| @@ -181,33 +181,33 @@ func (t *Trace) logInfo(threshold time.Duration) (string, []zap.Field) { | |||
| var steps []string | |||
There was a problem hiding this comment.
Hi, Line 207 is still using it~
There was a problem hiding this comment.
True
step, steps, tstep, t.steps.
Think this function needs a for major redo.
There was a problem hiding this comment.
I use tstep here based on t.steps. I mark it as TODO if need.
fuweid
force-pushed
the
fix-shadow-vet
branch
from
b703656
to
7dfe88d
Compare
etcdutl/snapshot/v3_snapshot.go
Outdated
| if err := b.ForEach(func(k, v []byte) error { | ||
| if _, err := h.Write(k); err != nil { | ||
| if err = b.ForEach(func(k, v []byte) error { | ||
| if _, err = h.Write(k); err != nil { |
There was a problem hiding this comment.
The err (line 152) overwrites/updates the err outside closure (line 151) directly? I know it works, but it looks a little strange. Can we rename it, e.g. berr? Same below
There was a problem hiding this comment.
fuweid
force-pushed
the
fix-shadow-vet
branch
from
7dfe88d
to
ae19589
Compare
| resp, err := cc.Get(ctx, "/") | ||
| if err != nil { | ||
| return err | ||
| resp, gerr := cc.Get(ctx, "/") |
There was a problem hiding this comment.
The renaming looks good to me. But why do you follow a different approach in other places? For example,
- in tests/robustness/traffic/traffic.go, you define
var c *RecordingClientin order to reuse theerrvariable. - tools/etcd-dump-logs/main.go, it's even worse. You overwrites/updates the
errdefined outside the switch statement. - lots of other similar cases.
Usually simpler is better. In this example, renaming the err variable in a small scope (such as here gerr) is better than defining a new resp variable. So I suggest to follow the same approach for all other places as well.
There was a problem hiding this comment.
Updated with some comments. PTAL.
fuweid
force-pushed
the
fix-shadow-vet
branch
from
ae19589
to
496051e
Compare
| if err != nil { | ||
| m.logAndCloseWithError(err, pw) | ||
| return | ||
| } | ||
| for { | ||
| resp, err := ss.Recv() | ||
| sresp, err := ss.Recv() |
There was a problem hiding this comment.
use new sresp here because the old name shadows the L240.
./maintenance.go:253:4: declaration of "resp" shadows declaration at line 240
| ctx, cancel := context.WithTimeout(context.Background(), s.Cfg.ReqTimeout()) | ||
| resp, lastErr := HashByRev(ctx, s.cluster.ID(), cc, ep, rev) | ||
| resp, lastErr = HashByRev(ctx, s.cluster.ID(), cc, ep, rev) |
There was a problem hiding this comment.
create new var to prevent shadow the lastErr.
etcdserver/corrupt.go:481:10: declaration of "lastErr" shadows declaration at line 478
| @@ -49,7 +49,9 @@ func NewTmpWAL(t testing.TB, reqs []etcdserverpb.InternalRaftRequest) (*wal.WAL, | |||
| if err != nil { | |||
| t.Fatalf("Failed to open WAL: %v", err) | |||
| } | |||
| _, state, _, err := w.ReadAll() | |||
|
|
|||
| var state raftpb.HardState | |||
There was a problem hiding this comment.
Creating new var state to reduce the line changes. err doesn't reuse in other goroutines. using err here might be more easy to review.
fuweid
force-pushed
the
fix-shadow-vet
branch
from
496051e
to
54b121a
Compare
Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
force-pushed
the
fix-shadow-vet
branch
from
54b121a
to
5e3910d
Compare
Part of #15549
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.