chore: Fix linter problems on master. #8930
Conversation
ajnavarro
changed the title
I'd probably just add linter ignore messages for these so you can unblock and merge #8928. We can then circle back to resolving them (especially for cross-package issues such as with go-namesys). |
core/coreapi/name.go
Outdated
| type ctxKey int | ||
|
|
||
| const ( | ||
| CtxKeyIpnsPublishTTL ctxKey = iota | ||
| ) |
There was a problem hiding this comment.
The standard pattern I've seen uses typed empty structs such as in https://github.com/libp2p/go-libp2p-kad-dht/blob/ee31a1490184f7429818c2787028301aa89a8cda/events.go#L230
There was a problem hiding this comment.
I just added linter ignore messages.
ajnavarro
force-pushed
the
fix/linter-on-master
branch
2 times, most recently
from
4adac10
to
47f4f16
Compare
commands/reqlog.go
Outdated
| rle.ID = rl.nextID | ||
| rl.nextID++ | ||
| rl.Requests = append(rl.Requests, rle) | ||
|
|
||
| if rle == nil || !rle.Active { | ||
| rl.maybeCleanup() | ||
| } | ||
|
|
||
| rle.ID = rl.nextID | ||
| rl.nextID++ | ||
| rl.Requests = append(rl.Requests, rle) | ||
| } |
There was a problem hiding this comment.
IIUC this doesn't really fix what the linter was complaining about it just tricks it into being happy.
(SA5011)[https://staticcheck.io/docs/checks#SA5011] is complaining because we did a nil check after we used a reference to the object. However, if rle was actually nil then we'd get a panic here anyway since calling rl.maybeCleanup() doesn't really change anything.
We should either explicitly handle the nil case (e.g. check if nil then emit a log and return), or just remove the nil check which wasn't previously helping us anyway.
There was a problem hiding this comment.
Being consistent with the rest of the methods, I would say that removing the nil check is ok. WDYT?
There was a problem hiding this comment.
Seems reasonable to me, it's certainly no worse than what we currently have. We may want to revisit what's going on with the ReqLog later anyway because I see that the code is duplicated here and in https://github.com/ipfs/go-ipfs-cmds/blob/master/reqlog.go and probably one of these should be deleted.
However, we can do this further down the road
ajnavarro
force-pushed
the
fix/linter-on-master
branch
from
47f4f16
to
4dd1a24
Compare
It looks like go linter was not executed previously. Fix some problems found on the master branch.
I have some concerns about context keys. If they were used somewhere we are breaking backward compatibility.
Signed-off-by: Antonio Navarro Perez antnavper@gmail.com