-
Notifications
You must be signed in to change notification settings - Fork 965
Log Rewards errors by default and add a feature flag for verbose logging #8598
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -790,7 +790,7 @@ class RewardsServiceImpl : public RewardsService, | |
bool reset_states_; | ||
bool ledger_for_testing_ = false; | ||
bool resetting_rewards_ = false; | ||
bool should_persist_logs_ = false; | ||
int persist_log_level_ = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that more logging would be ideal. Unfortunately, logging at 2 will create for us a dilemma: we'll likely have to remove useful information from messages at those levels, which will make those levels less useful for development and detailed debugging. I would prefer that we stay with logging only errors by default in this PR, and consider increasing the default log level as a future change. |
||
|
||
GetTestResponseCallback test_response_callback_; | ||
|
||
|
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.
Maybe worth checking the performance impact on Android?
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.
cc @deeppandya
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 don't think it would have any impact on performance. I can run through the branch and quickly check.