chore(core): Resolve non-actionable TODOs - #6826
Merged
Merged
Conversation
Three TODOs that describe work that won't happen as written, verified and resolved: - sentryMetroSerializer.ts: drop the "deprecate then rename to createSentryDebugIdSerializer" TODO. It's a cosmetic rename of a public (`@sentry/react-native/metro`) export that is now used internally by `withSentryConfig`; the rename would need a full deprecation/breaking cycle for no functional gain. - transports/native.ts: reframe the TODO as a limitation note. The native SDK owns the transport and `captureEnvelope` returns only a boolean, so there is no HTTP response to surface to JS. - RNSentryNativeLogsForwarder.m: reframe the TODO as a limitation note. cocoa exposes no public getter for the current SentrySDKLog output (the only getter is test-only, `#if SENTRY_TEST`), so the previous block can't be restored; resetting to the default NSLog behavior is the only option. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bc33828. Configure here.
antonis
marked this pull request as ready for review
October 2, 2026 10:38
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
tools/sentryMetroSerializer.tsdeprecate this and afterwards rename to createSentryDebugIdSerializer@sentry/react-native/metro) export now used internally bywithSentryConfig. Would need a full deprecation/breaking cycle for no functional gain.transports/native.tsWe currently can't retrieve the response information from nativecaptureEnvelopereturns only a boolean, so there's no HTTP response to surface.ios/RNSentryNativeLogsForwarder.mIdeally we should save the previous output block … instead of hardcoding NSLogSentrySDKLogoutput, so the previous block can't be restored.💡 Motivation and Context
TODO cleanup pass. Verified rather than assumed:
💚 How did you test it?
N/A
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps