add go linter - "unused" #11235
add go linter - "unused" #11235
Conversation
| @@ -32,6 +32,8 @@ import ( | |||
| ) | |||
|
|
|||
| // recordPoStFailure records a failure in the journal. | |||
| // | |||
| //nolint:unused | |||
There was a problem hiding this comment.
are these false positives?
There was a problem hiding this comment.
Yes, they're called over an API.
| @@ -32,6 +32,8 @@ import ( | |||
| ) | |||
|
|
|||
| // recordPoStFailure records a failure in the journal. | |||
| // | |||
| //nolint:unused | |||
There was a problem hiding this comment.
Yes, they're called over an API.
| "testing" | ||
| ) | ||
|
|
||
| func TestQueue(t *testing.T) { |
There was a problem hiding this comment.
No objection to this test, obviously, but I'm surprised it was needed to make the queue lok "used" -- it should already look used because of its use in rate limiting.
Do you understand why this was the case?
There was a problem hiding this comment.
Yes! I sort of understand, for some reason pop is unused:
chain/sub/ratelimit/queue.go:38:17: func `(*queue).pop` is unused (unused)
func (q *queue) pop() int64 {
it's pretty common to write a little datastructure library, write out a bunch of api/funcs for it, and then not use a few when you have your final implementation. I think there's some combination of peeking and truncating that happens in ./chain/sub/ratelimit/window.go instead of pop, but i haven't done a thorough dive into it. Given my expectation of this pattern of building a general tool and then just using some of it, the test made more sense than deleting pop in case in future we wanted to reorganize this code. also just a simple parallel example, might make sense to write a queue library that lets you take out values from the top or bottom, but so far only use one of them
There was a problem hiding this comment.
Got it, thanks for the explanation! Makes total sense.
Related Issues
when we upgraded go versions, it was pointed out to me that "unused" is a recommended linter. Adding unused triggered a few errors regarding unused close and I clean them up here
Proposed Changes
Additional Info
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>fix: mempool: Introduce a cache for valid signaturesPR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, testarea, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps