-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Reduce server log verbosity #53268
Conversation
While arguably educational, this warning increases the amount of noise within the server log, which can make it difficult to debug errors, warnings, or intentionally logged values.
While informative, this un-filterable log increases the amount of noise in the server log, which can make it difficult to debug errors, warnings, or intentionally logged values. Additionally, the Hermes engine is now globally enabled for both iOS and Android.
This warning appears to no longer occur, presumably thanks to: #52563
This warning appears to no longer occur. The configuration was originally added in: a1b11c5
Size Change: 0 B Total Size: 1.44 MB ℹ️ View Unchanged
|
I realize now this statement is untrue for the Demo editor. If we'd like to reinstate the log for the time being, that is understandable. Although, I see no harm in removing it. |
Flaky tests detected in f4589cb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5739020441
|
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 🎊 !
const isHermes = () => global.HermesInternal !== null; | ||
// eslint-disable-next-line no-console | ||
console.log( 'Hermes is: ' + isHermes() ); |
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.
It might be helpful to know if Hermes is enabled, although I agree logging this here is not the best approach. Eventually, it would be great to have a place to display these values.
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.
What benefit or action might come of knowing Hermes is enabled? I do not ask to question the value, but to better understand.
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.
It might be interesting when debugging and changing between running the JS code in the app or the local debugger, especially for the bugs that only happen when running Hermes. It's a bit of an edge case, as I think it only happened a couple of times.
What?
Reduce the number of un-filterable server logs and warnings.
Why?
Un-filterable logs and warnings create ever-present noise that increases the
likelihood a new warning is overlooked and inhibits the ability to use the
server log for ad-hoc debugging.
How?
refactor: Remove outdated LogBox configuration for gesture handler
This warning appears to no longer occur. The configuration was
originally added in: a1b11c5
refactor: Remove outdated LogBox configuration for LayoutAnimation
This warning appears to no longer occur, presumably thanks to:
#52563
refactor: Remove Hermes engine status log
While informative, this un-filterable log increases the amount of noise
in the server log, which can make it difficult to debug errors,
warnings, or intentionally logged values.
refactor: Remove unsupported line height Aztec warning
While arguably educational, this warning increases the amount of noise
within the server log, which can make it difficult to debug errors,
warnings, or intentionally logged values.
Testing Instructions
Launch the Demo editor and verify no unexpected logs and warnings are shown in
the server log.
Testing Instructions for Keyboard
n/a
Screenshots or screencast
n/a