golangci-lint: add unconvert #11757
golangci-lint: add unconvert #11757
Conversation
|
I see that you haven't updated any README files. Would it make sense to do so? |
jmank88
force-pushed
the
golangci-lint-unconvert
branch
4 times, most recently
from
58fe6f7
to
87f2b46
Compare
jmank88
force-pushed
the
golangci-lint-unconvert
branch
from
87f2b46
to
868ab29
Compare
jmank88
requested review from
jkongie,
bolekk,
a team,
samsondav,
sdrug,
martin-cll,
connorwstein,
prashantkumar1982,
krehermann,
RyanRHall,
PiotrTrzpil and
reductionista
as code owners
|
SonarQube Quality Gate
|
| @@ -236,7 +236,7 @@ func (f *FwdMgr) runLoop() { | |||
| defer f.wg.Done() | |||
| tick := time.After(0) | |||
|
|
|||
| for ; ; tick = time.After(utils.WithJitter(time.Duration(time.Minute))) { | |||
| for ; ; tick = time.After(utils.WithJitter(time.Minute)) { | |||
There was a problem hiding this comment.
Unrelated to the scope of this PR - is there a more idiomatic way to write this?
There was a problem hiding this comment.
It does seem odd. Moving the assignment to the body isn't straightforward though (extra bool might be most elegant?).
I might actually prefer moving the init here:
| for ; ; tick = time.After(utils.WithJitter(time.Minute)) { | |
| for tick := time.After(0); ; tick = time.After(utils.WithJitter(time.Minute)) { |
Or, since this is a common pattern that we use instead of a plain time.Ticker, we could just make a custom type of Ticker which sends the first tick without delay. The jitter can become an implementation detail this way too:
| for ; ; tick = time.After(utils.WithJitter(time.Minute)) { | |
| ticker := services.NewTicker(time.Minute) | |
| defer ticker.Stop() | |
| for { |
There was a problem hiding this comment.
Here is a quick hack: smartcontractkit/chainlink-common#311
| c.lock.RLock() | ||
| defer c.lock.RUnlock() | ||
|
|
||
| i := int64(block) % int64(c.cap) | ||
| i := int64(block) % c.cap | ||
| b := c.buffer[i] | ||
| if b.block != block { | ||
| return ocr2keepers.TransmitEvent{}, false |
There was a problem hiding this comment.
/nit This feels more readable to me
func (c *transmitEventCache) get(block int64, logID string) (ocr2keepers.TransmitEvent, bool) {
c.lock.RLock()
defer c.lock.RUnlock()
i := block % c.cap // no longer has an int64 cast on block
b := c.buffer[i]
if int64(b.block) != block { //has an int64 cast on b.block
return ocr2keepers.TransmitEvent{}, false
}
if len(b.records) == 0 {
return ocr2keepers.TransmitEvent{}, false
}
e, ok := b.records[logID]
return e, ok
}
I'm not sure on the tradeoff with this pattern:
type BlockNumber uint64
What's the readability/usability gain relative to all the casting when we need to perform an int64 operation?
There was a problem hiding this comment.
I don't know this code, but that makes sense to me.
| @@ -74,7 +74,7 @@ var _ Task = (*BridgeTask)(nil) | |||
|
|
|||
| var zeroURL = new(url.URL) | |||
|
|
|||
| const stalenessCap = time.Duration(30 * time.Minute) | |||
There was a problem hiding this comment.
Should this have a comment explaining where it comes from or be wired through config?
There was a problem hiding this comment.
It does seem arbitrary. Introduced here: #7964
No description provided.