SQLDelight integration #3
Conversation
wzieba
force-pushed
the
local_database
branch
2 times, most recently
from
77b55ed
to
80cfe8a
Compare
| } | ||
|
|
||
| val iosFrameworkName = "shared" | ||
| val kotlinVersion = "1.4.3-native-mt" | ||
| val kotlinVersion = "1.4.2-native-mt" |
There was a problem hiding this comment.
Blocked by Turbine cashapp/turbine#29
shared/build.gradle.kts
Outdated
| val iOSTarget: (String, KotlinNativeTarget.() -> Unit) -> KotlinNativeTarget = | ||
| if (System.getenv("SDK_NAME")?.startsWith("iphoneos") == true) | ||
| ::iosArm64 | ||
| else | ||
| ::iosX64 |
There was a problem hiding this comment.
This should be handled with Hierarchical Project Structure but when I look for Kotlin targets in packForXcode task, I still see native targets:
{android=target android (androidJvm), iosArm64=target iosArm64 (native), iosX64=target iosX64 (native), metadata=target metadata (common)}
This might be a problem with documentation or I don't understand something. I plan to create an issue for that on KMM documentation.
| matching { | ||
| it.name.endsWith("Test") | ||
| }.configureEach { | ||
| languageSettings.useExperimentalAnnotation("kotlin.time.ExperimentalTime") |
There was a problem hiding this comment.
We use Turbine which uses Kotlin's Duration.
| event_name = tracksEvent.name, | ||
| user = "", | ||
| user_agent = null, | ||
| timestamp = null, | ||
| retry_count = null, | ||
| user_type = null, | ||
| user_props = null, | ||
| device_info = null, | ||
| custom_props = null |
There was a problem hiding this comment.
this structure is taken from current implementation of Tracks, I wasn't sure if it's all needed, probably we'll verify that later.
| import kotlinx.coroutines.runBlocking | ||
| import kotlin.test.assertTrue | ||
|
|
||
| internal class DatabaseTests { |
There was a problem hiding this comment.
The whole idea for DatabaseTests is to test SQLDelight on each platform (Android and iOS) with its native driver.
We have to create a native driver in platform-specific source sets as it requires platform specific dependencies, like Context in Android. So we create it there and then run DatabaseTests.
The cons of this approach are less readable test results as we technically have only one @Test (that will run multiple tests) but we can see which test failed in the stacktrace which is not the worse thing ever.
The other idea would be to create a new, test-only multiplatform module like KaMPKit does, but it'd require us to remove internal from classes we want to be internal and this would cause a lot of problems for consumers I believe.
|
I'll merge this @0nko , please feel free though to add any comments you have after merge - I'll apply improvements in next PRs |
| remoteRepository.send(event) | ||
| localRepository.observe().collect { events -> | ||
| if (events.isNotEmpty()) { | ||
| remoteRepository.send(events.first()) |
There was a problem hiding this comment.
Why just the first? Shouldn't we iterate over all events?
There was a problem hiding this comment.
The idea I was having was to pick the first (or e.g. "the most important") event, process it, send and after successful sending - remove it from DB which will trigger the observe again.
But we could e.g. order events here from "the most important" to the less and then send whole list to RemoteRepository - both works fine for me.
"most important" - TBD, maybe number of failed attempts or something like this
No description provided.