-
Notifications
You must be signed in to change notification settings - Fork 170
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
Upgrade Kotlin to v2.0 #3594
Upgrade Kotlin to v2.0 #3594
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3594 +/- ##
===========================================
+ Coverage 82.77% 82.82% +0.05%
===========================================
Files 1747 1747
Lines 41391 41742 +351
Branches 5015 5102 +87
===========================================
+ Hits 34261 34573 +312
+ Misses 5358 5355 -3
- Partials 1772 1814 +42 ☔ View full report in Codecov by Sentry. |
Depends on element-hq/matrix-rich-text-editor#24 to build without warnings. |
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
I believe this is ready to test now. |
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.
Thanks! 2 non-blocking remarks.
/** | ||
* A recorder that can be used to record the parameters of a suspend lambda invocation. | ||
*/ | ||
abstract class LambdaSuspendRecorder internal constructor( |
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 would not add all those classes and fun, but rather fix the lambda type so that they do not expect a suspend
qualifier.
Worth case we can use runBlocking
in the lambda body (this are tests, not production code) if this is calling a suspend fun. There is only one case like this AFAICT.
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.
Good call. At first I though it wouldn't be possible since I was getting other errors related to NoSuchMethodError
but it seems like using runBlocking
properly does help.
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.
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.
Thanks. Let me check why it was necessary to add return runBlocking { block() }
in all the Lambda*Recorder
classes. Since block
does not suspend, I do not see how it helps. I was thinking about adding it on demand at the call site.
@@ -117,4 +117,8 @@ | |||
|
|||
<!-- Compose --> | |||
<issue id="UnnecessaryComposedModifier" severity="error" /> | |||
|
|||
<!-- There seems to be an issue with this check, it flags lots of false positives. --> | |||
<!-- TODO: check again in the near future. --> |
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 add a link to https://issuetracker.google.com/issues/349411310 ?
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.
5cd4770
to
1ef1ca2
Compare
listOf(value(MobileScreen.ScreenName.RoomCall)), | ||
listOf(value(MobileScreen.ScreenName.RoomCall)) | ||
) | ||
analyticsLambda.assertions().isCalledOnce().with(value(MobileScreen.ScreenName.RoomCall)) |
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.
Interesting that this "bug" is fixed. Do you have any idea why?
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'm not sure to be honest. I originally changed this code some time ago 😓 .
Co-authored-by: Benoit Marty <benoit@matrix.org>
Quality Gate passedIssues Measures |
Content
v2.0.20
.setupAnvil()
method in some gradle modules that added the anvil plugin manually instead.LambdaSuspendRecorder
since Kotlin 2.0+ differentiates between a() -> Unit
lambda and asuspend () -> Unit
so we need a special one for suspend lambdas.Motivation and context
Keeping the dev env up to date.
Tests
Let's keep an eye on the CI.
The app runs fine when built locally.
Tested devices
Checklist