-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add feature handle-panics
#525
Add feature handle-panics
#525
Conversation
return; | ||
} | ||
|
||
if let Some(hook) = unsafe { DEFAULT_HOOK.as_ref() } { |
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 think we might want to add #[allow(static_mut_refs)]
right above this to eliminate that warning.
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 think it's worth more to run full fmt+clippy check as a separate PR. There are too many warnings besides this one.
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.
looks good, i'm in favor of having a feature flag for this for now since some people may rely on the output of panics still and not want them suppressed. when we ship the other pr in i may be in favor of removing the ff since we'll need a major version bump
When
handle-panics
is enabled, panic backtrace which happens in test case is silenced instead of spitting it to consoleThis PR is part of #421/#517 which doesn't change public API. Part with backtraces capture will be presented as separate PR
P.S: Do we actually need feature to gate this out? Or should it be just enabled by
std
?