-
Notifications
You must be signed in to change notification settings - Fork 54
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
refactor: Change logging calls to use feedback in CLI package #714
Conversation
935d515
to
747e8f9
Compare
Codecov Report
@@ Coverage Diff @@
## develop #714 +/- ##
========================================
Coverage 57.42% 57.43%
========================================
Files 143 143
Lines 16408 16406 -2
========================================
Hits 9422 9422
+ Misses 6102 6100 -2
Partials 884 884
|
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.
n.Close() //nolint:errcheck | ||
db.Close(cmd.Context()) | ||
return err | ||
return fmt.Errorf("failed to start P2P node: %w", err) |
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.
question: We remove logging here because we will log it once it bubble ups?
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.
yes 🙂
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
…network#714) Relevant issue(s) Resolves sourcenetwork#662 Description This PR changes all logging calls in the CLI package to include feedback.
Relevant issue(s)
Resolves #662
Description
This PR changes all logging calls in the CLI package to include feedback.
Tasks
How has this been tested?
manual check
Specify the platform(s) on which this was tested: