-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: Add aggressive linting with golangci-lint #129
Conversation
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.
Much cleaner 🚀
@@ -34,7 +34,7 @@ func (sc *SentCheckpoints) ShouldSend(epoch uint64) bool { | |||
return true | |||
} | |||
// 2. should resend if some interval has passed since the last sent | |||
durSeconds := uint(time.Now().Sub(*ckptInfo.ts).Seconds()) | |||
durSeconds := uint(time.Since(*ckptInfo.ts).Seconds()) |
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.
Wow, the linter can even identify this optimisation! 👏
@@ -24,8 +24,6 @@ type subscriptions struct { | |||
zfront *zmq.Socket | |||
latestEvent time.Time | |||
active bool | |||
|
|||
sequence []chan *SequenceMsg |
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.
Didn't realise that we are not using this field. 😢 What was it used for? @gusin13
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.
I realized this got left in refactoring, we were using it before but then started using another channel to publish messages blockEventChan
.
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.
Cool!
) | ||
|
||
func readCAFile(cfg *config.BTCConfig) []byte { | ||
// Read certificate file if TLS is not disabled. | ||
if !cfg.DisableClientTLS { | ||
certs, err := ioutil.ReadFile(cfg.CAFile) | ||
certs, err := os.ReadFile(cfg.CAFile) |
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 curious why this needs to be replaced
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.
ioutil
is deprecated https://pkg.go.dev/io/ioutil
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.
LGTM 👍
Add linting similar to the Babylon repository to the vigilante. There were some unused variables that were removed (or used), so please pay attention to those (especially for zmq).