chore(core): resolve stale TODO comments - #6819
Merged
Merged
Conversation
Resolve four stale/answerable TODOs after verifying each against the source: - reactnavigation.ts / reactnativenavigation.ts "What if it's not SentrySpan?": the latest navigation span is always a SentrySpan or a SentryNonRecordingSpan, and the latter's end() is a no-op, so there is nothing to discard. Replaced with a one-line note at the guard. - reactnativenavigation.ts "Should we include pass props?": passProps is arbitrary app-defined data (potential PII), so it is intentionally omitted. Documented as a one-line note so it is not added back unguarded. - RNSentry.mm "If the callback isn't executed the promise wouldn't be resolved": stale — resolve() is unconditional, and cocoa's configureScope runs the block synchronously when the SDK is initialized (nil scope otherwise, which the JS consumer already handles). Removed the stale comment. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
antonis
marked this pull request as ready for review
October 1, 2026 13:34
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. |
alwx
approved these changes
Oct 1, 2026
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
Resolves four stale or answerable
TODOcomments after verifying each against the source. Comment-only — no behavior change.tracing/reactnavigation.ts,tracing/reactnativenavigation.ts// TODO: What if it's not SentrySpan?SentrySpanor aSentryNonRecordingSpan; the latter'send()is a no-op, so there's nothing to discard. Replaced with a one-line note at the guard.tracing/reactnativenavigation.ts// TODO: Should we include pass props?passPropsis arbitrary app-defined data (potential PII), so it's intentionally omitted. Documented so it isn't added back unguarded.ios/RNSentry.mm// TODO: If the callback isn't executed the promise wouldn't be resolved.resolve()is unconditional. Removed the comment.💡 Motivation and Context
TODO cleanup pass
💚 How did you test it?
yarn lint(oxlint + format) — cleanclang-formatonRNSentry.mm— cleanyarn jestnavigation suites — 135 passing📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps