-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
static check fixes #1076
static check fixes #1076
Conversation
@@ -247,7 +247,15 @@ func (ar *AutoRelay) discoverRelays(ctx context.Context) ([]peer.AddrInfo, error | |||
|
|||
ctx, cancel := context.WithTimeout(ctx, 30*time.Second) | |||
defer cancel() | |||
return discovery.FindPeers(ctx, ar.discover, RelayRendezvous, discovery.Limit(1000)) | |||
var ret []peer.AddrInfo | |||
ch, err := ar.discover.FindPeers(ctx, RelayRendezvous, discovery.Limit(1000)) |
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 should be equivilent to the FindPeers util function in go-libp2p-discovery, to convert the channel into a slice.
@@ -20,7 +20,31 @@ func Advertise(ctx context.Context, advertise discovery.Advertiser) { | |||
go func() { | |||
select { | |||
case <-time.After(AdvertiseBootDelay): | |||
discovery.Advertise(ctx, advertise, RelayRendezvous, discovery.TTL(AdvertiseTTL)) | |||
go func() { |
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 is brought in from go-libp2p-discovery.
If it's better to use the Util Functions from there I can swtich back to using them.
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.
Well, those functions exist so we don't need to duplicate this code. What was the motivation for this change?
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.
Ah... I see.
p2p/protocol/identify/id.go
Outdated
@@ -368,7 +368,7 @@ func (ids *IDService) identifyConn(c network.Conn, signal chan struct{}) { | |||
|
|||
// ok give the response to our handler. | |||
if err = msmux.SelectProtoOrFail(ID, s); err != nil { | |||
log.Event(context.TODO(), "IdentifyOpenFailed", c.RemotePeer(), logging.Metadata{"error": err}) | |||
// log.Event(context.TODO(), "IdentifyOpenFailed", c.RemotePeer(), logging.Metadata{"error": err}) |
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.
Is there a metric that should be emitted here in leiu of depreciated go-log Events?
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.
Just a structured log message.
p2p/host/routed/routed.go
Outdated
@@ -131,7 +130,7 @@ func logRoutingErrDifferentPeers(ctx context.Context, wanted, got peer.ID, err e | |||
lm["error"] = err | |||
lm["wantedPeer"] = func() interface{} { return wanted.Pretty() } | |||
lm["gotPeer"] = func() interface{} { return got.Pretty() } | |||
log.Event(ctx, "routingError", lm) | |||
// log.Event(ctx, "routingError", lm) |
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.
Why is this code commented?
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 entire function should be deleted and replaced with a call to log.Errorw
.
Side note: please (almost) never comment out code unless you're actively debugging.
p2p/net/mock/mock_test.go
Outdated
} | ||
return p | ||
} | ||
// Unused code: |
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.
Can we delete this?
@@ -20,7 +20,31 @@ func Advertise(ctx context.Context, advertise discovery.Advertiser) { | |||
go func() { | |||
select { | |||
case <-time.After(AdvertiseBootDelay): | |||
discovery.Advertise(ctx, advertise, RelayRendezvous, discovery.TTL(AdvertiseTTL)) | |||
go func() { |
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.
Well, those functions exist so we don't need to duplicate this code. What was the motivation for this change?
p2p/host/routed/routed.go
Outdated
@@ -131,7 +130,7 @@ func logRoutingErrDifferentPeers(ctx context.Context, wanted, got peer.ID, err e | |||
lm["error"] = err | |||
lm["wantedPeer"] = func() interface{} { return wanted.Pretty() } | |||
lm["gotPeer"] = func() interface{} { return got.Pretty() } | |||
log.Event(ctx, "routingError", lm) | |||
// log.Event(ctx, "routingError", lm) |
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 entire function should be deleted and replaced with a call to log.Errorw
.
Side note: please (almost) never comment out code unless you're actively debugging.
fc3ffcc
to
80ee4cd
Compare
80ee4cd
to
1f5f748
Compare
1f5f748
to
2ad02f7
Compare
various fixes for staticcheck
For context: protocol/.github#41