-
Notifications
You must be signed in to change notification settings - Fork 56
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
to 1.1.30: feat: add consistent logging at INFO and DEBUG levels #930
base: v1.1.30
Are you sure you want to change the base?
Conversation
The goal is to make viewing the logs at the INFO level nice when manually reviewing log output and to make the DEBUG output comprehensive for everything except escrow logging.
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.
Same comment and request as the other log PRs
Most log messages are unique enough that a simple search of the code base finds the one you are looking for. And if they aren't, I'd rather update the few that are to be more specific / descriptive and avoid this bespoke cataloging of log messages.
Yes, while this is accurate, it was very useful to me while debugging multisig to be able to read the log output without having to go search. The reason I added the bespoke cataloging of log messages was because the bespoke labels added context that helped me think through the problem without having to constantly search the codebase. IMO it's a set of tradeoffs. Add a bit more length to log messages, gain the ability to quickly understand what is going on. As the person who wrote many, if not most of them, them I can see why you would lean in favor of avoiding the bespoke labels because you have memorized the context and can follow the logic often without having to refer back to the codebase. As a newcomer to the codebase I found it tremendously helpful to have that specific tagging in the label. I imagine such labels will help other newcomers quickly understand the meaning of the log messages. |
d41d53a
to
9844b9d
Compare
In the latest commit I addressed the class name prefix comment and metadata comments from this morning's dev meeting. The class names are removed and the metadata format string is removed as library users will customize that themselves. |
1fa72be
to
96b095b
Compare
96b095b
to
47f53de
Compare
The goal is to make viewing the logs at the INFO level nice when manually reviewing log output and to make the DEBUG output comprehensive for everything except escrow logging.