-
Notifications
You must be signed in to change notification settings - Fork 116
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
Make alerting package stateful and directly error-logging. Fix the rest of the unchecked errors. #505
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.
LGTM, have a few error messages to embellish but they won't block the PR; they can come later with the master merge.
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.
Added said embellishments in as comments.
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.
Cheers!
c61a133
to
a2c37f1
Compare
The
alerting
package previously was stateless and simply recreated state every time any alert was called.Now it is stateful, and only checks config on init(). Logging is done through a logger instead of through stdout.
Fixes
errcheck
issues and uses newestrocketpool-go/v2
andnode-manager-core
commits which resolve upstream errcheck issues.Enables all linters 🎉