-
Notifications
You must be signed in to change notification settings - Fork 160
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
Room : makes subscribeToSync/unsubscribeFromSync suspendable #1457
Room : makes subscribeToSync/unsubscribeFromSync suspendable #1457
Conversation
…s sure we keep subscription count
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #1457 +/- ##
===========================================
- Coverage 57.92% 57.85% -0.07%
===========================================
Files 1129 1130 +1
Lines 30046 30080 +34
Branches 6144 6148 +4
===========================================
Hits 17404 17404
- Misses 10031 10065 +34
Partials 2611 2611
☔ View 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.
LGTM, thanks. Maybe add an entry for the changelog? Not sure since it does not change anything for the user, but it may help to investigate regression (if any!)
Kudos, SonarCloud Quality Gate passed! |
Rework how we use subscribe/unsubscribe on sync.
Previously the calls were synced and on the main thread, which could lead to break the navigation animation or even causes some ANR.
This is now handled properly with suspendable, and we keep a subscription count to avoid messing with the asynchronous nature of node lifecycle and coroutines.