sentry-panic: switch to PanicHookInfo from deprecated PanicInfo#1074
sentry-panic: switch to PanicHookInfo from deprecated PanicInfo#1074mvanhorn wants to merge 1 commit intogetsentry:masterfrom
PanicHookInfo from deprecated PanicInfo#1074Conversation
The workspace MSRV is 1.88, so `std::panic::PanicHookInfo` is always available. Drop the six `#[allow(deprecated)]` annotations that were silencing the `PanicInfo` deprecation warning and use `PanicHookInfo` directly. Source compatibility is preserved because `std` keeps `PanicInfo` as a type alias for `PanicHookInfo` (see `library/std/src/panic.rs`), so downstream extractor closures that take `&PanicInfo<'_>` continue to match the new `&PanicHookInfo<'_>` signatures. Closes getsentry#862.
|
This PR has been automatically closed. The referenced issue does not show a discussion between you and a maintainer. To avoid wasted effort on both sides, please discuss your proposed approach in the issue first and wait for a maintainer to respond before opening a PR. Please review our contributing guidelines for more details. |
|
Sorry about the bot @mvanhorn, this PR indeed looks valid. I'll review when I have time. In general, we like issues to be opened before PRs, but for simple stuff like this change, I think the issue is not necessarily needed. Thanks again for your contribution! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1074 +/- ##
==========================================
+ Coverage 73.81% 73.85% +0.03%
==========================================
Files 64 65 +1
Lines 7538 7557 +19
==========================================
+ Hits 5564 5581 +17
- Misses 1974 1976 +2 |
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Ok, just did a quick review; I am not sure whether we should do this at all, since it would break our public API.
Unless using PanicInfo is causing you some inconvenience, I would suggest we just keep the code as it is for now. Otherwise, perhaps it makes sense to open an issue to discuss the right approach after all 😅
If using PanicInfo is causing trouble for you, please open an issue to explain what the concrete problem is, and we can discuss a potential solution there.
|
Thanks for the quick look @szokeasaurusrex. The PR body covers this but may have buried it - Happy to close if you'd still rather leave things as-is - the deprecation warning is cosmetic and the allow attrs aren't hurting anything. Just didn't want you to have to merge another six |
Closes #862.
The workspace MSRV is 1.88 (
Cargo.toml), sostd::panic::PanicHookInfo(stabilized in 1.81) is always available. This removes the six#[allow(deprecated)]annotations that were silencing thePanicInfodeprecation warning and usesPanicHookInfodirectly insentry-panic.Source compatibility
stdkeepsPanicInfoas a type alias forPanicHookInfo:So downstream extractor closures that still use
&PanicInfo<'_>continue to match the new&PanicHookInfo<'_>signatures onadd_extractor,event_from_panic_info, andmessage_from_panic_info.Scope
Only
sentry-panic/src/lib.rsis touched. No other crates referencePanicInfo.Verified with
cargo build --all,cargo clippy -p sentry-panic,cargo fmt -p sentry-panic --check, andcargo test -p sentry-panic.