-
Notifications
You must be signed in to change notification settings - Fork 89
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(2523): Improve logging, primarily on desktop #2518
Conversation
Resolves #2523 |
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 would prefer to see the rest of our console.log
statements brought into the new paradigm and for clarity on my comments before we merge just so that we don't end up accidentally walking down the multiple logging standards path longer than we'd like.
@@ -0,0 +1,20 @@ | |||
import { shouldPolyfill as shouldPolyfillCanon } from '@formatjs/intl-getcanonicallocales/should-polyfill' |
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'm not sure if this is the best place for this polyfill. I wanted it to be in mobile since its specific to mobile but the mobile code wasn't being run soon enough, it seems, to run before the logging started.
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 also added some doc strings to the this file for clarity.
QuietLogger
class with improved formatting and colorizationPull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: