-
Notifications
You must be signed in to change notification settings - Fork 20.5k
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
build: enable goimports and varcheck linters #16446
Changes from 16 commits
ebf6df0
e79ce54
8889632
c41a7cc
9671bfc
a1b426a
a48f2a4
dea717b
b2e72fc
f77e0b4
84c3ead
431af6b
ab0dcec
639a37d
db5d83a
10ad506
fe98d8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#!/usr/bin/env bash | ||
|
||
find_files() { | ||
find . -not \( \ | ||
\( \ | ||
-wholename '.github' \ | ||
-o -wholename './build/_workspace' \ | ||
-o -wholename './build/bin' \ | ||
-o -wholename './crypto/bn256' \ | ||
-o -wholename '*/vendor/*' \ | ||
\) -prune \ | ||
\) -name '*.go' | ||
} | ||
|
||
GOFMT="gofmt -s -w"; | ||
GOIMPORTS="goimports -w"; | ||
find_files | xargs $GOFMT; | ||
find_files | xargs $GOIMPORTS; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the purpose of this file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of the reasons of linting errors was due to having code not properly formatted and not properly imported. There was no standard set for that. Now there is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I started to work on this issue, I found that the devs are not really doing what you said is their job...
So I ran the goimports the way "I think" is the best way, splitting the imports in 3. Just to find out that this is not the proffered way here.
So I added a "default" script to get this sorted and leave no room for errors. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,8 +38,6 @@ import ( | |
const ( | ||
// chainHeadChanSize is the size of channel listening to ChainHeadEvent. | ||
chainHeadChanSize = 10 | ||
// rmTxChanSize is the size of channel listening to RemovedTransactionEvent. | ||
rmTxChanSize = 10 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please drop comment too. |
||
) | ||
|
||
var ( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,6 @@ import ( | |
var ( | ||
errInvalidEvent = errors.New("invalid in current state") | ||
errNoQuery = errors.New("no pending query") | ||
errWrongAddress = errors.New("unknown sender address") | ||
) | ||
|
||
const ( | ||
|
@@ -828,11 +827,10 @@ type nodeEvent uint | |
//go:generate stringer -type=nodeEvent | ||
|
||
const ( | ||
invalidEvent nodeEvent = iota // zero is reserved | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed another issue here: the nodeEvent String method is created by stringer. Please re-run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm running into a error when running stringer: p2p/discv5/database.go
p2p/discv5/database_test.go
p2p/discv5/net.go
stringer: checking package: database.go:23:2: could not import bytes (reading export data: /usr/local/go/pkg/linux_amd64/bytes.a: invalid encoding format in export data: got 'v'; want 'c' or 'd')
p2p/discv5/net.go:827: running "stringer": exit status 1 I tried to find a way out, but so far I found nothing, any ideas ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably need to rebuild your copy of stringer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fjl sorry for the delay, I got caught in other issue to work on. Just re-generated the nodeevent with stringer as you requested. |
||
|
||
// Packet type events. | ||
// These correspond to packet types in the UDP protocol. | ||
pingPacket | ||
pingPacket = iota + 1 | ||
pongPacket | ||
findnodePacket | ||
neighborsPacket | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,8 +47,6 @@ const ( | |
discMsg = 0x01 | ||
pingMsg = 0x02 | ||
pongMsg = 0x03 | ||
getPeersMsg = 0x04 | ||
peersMsg = 0x05 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are these unused? Are we sure this is correct here @fjl ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, they are unused |
||
) | ||
|
||
// protoHandshake is the RLP structure of the protocol handshake. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,7 +54,6 @@ const ( | |
|
||
// key prefixes for leveldb storage | ||
kpIndex = 0 | ||
kpData = 1 | ||
) | ||
|
||
var ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no
install-linters
step.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is removed in recent versions of the diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, it's not removed? It didn't show up for me until now.