-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
feat: Support orientation change for session replay #4194
Conversation
…entry/sentry-cocoa into feat(SR)/replay-for-crashes
…entry/sentry-cocoa into feat(SR)/replay-for-crashes
Co-authored-by: Andrew McKnight <andrew.mcknight@sentry.io>
…ry/sentry-cocoa into feat/support-size-change
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4194 +/- ##
=============================================
+ Coverage 91.350% 91.480% +0.129%
=============================================
Files 610 611 +1
Lines 48963 49251 +288
Branches 17596 17810 +214
=============================================
+ Hits 44728 45055 +327
+ Misses 4142 4103 -39
Partials 93 93
... and 45 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
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 have a couple of questions, almost LGTM.
Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift
Outdated
Show resolved
Hide resolved
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
…ry/sentry-cocoa into feat/support-size-change
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.
Maybe it makes sense to mention why we don't need to run anything async for resumePreviousSessionReplay
as crash events are captured from a background thread, as mentioned here #4194 (comment).
LGTM.
📜 Description
Session replay will adapt to screen size changes, that usually happens during orientation change.
💚 How did you test it?
Sample and Unit tests
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.🔮 Next steps