-
Notifications
You must be signed in to change notification settings - Fork 135
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
♻️ Convert Local module to KMP #535
Merged
Merged
Conversation
This file contains 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
A new `shared` module was create to hold all the shared logic between the different mobile platforms. Future changes will make the `shared` module the new `domain` if possible. Since the `:domain` module is already Kotlin-only, the code was simply moved and works!
Moved the dependencies from the Kotlin-only to the Kotlin Multiplatform Mobile one
For some reason, Kotlin-only modules do not find the KMM modules. This module was temporarily converted to Android to make the code work. In the future, this module will also be a KMM one.
Small back-and-forth to convert the `shared` to the old `domain`. Now that everything is set, the module was updated to the original name.
Oopsy, I removed the tests when moving the class between the modules. The classes are now back and removed the junit dependencies since it doesn't work well with KMM.
Removing Java's Calendar and adapting the logging tool to its multiplatform version. Some work was needed for the convertions since the Android layers will keep using Calendar for now. Future commits will address the TODOs and make it more clear.
Fix the logging system in the remaining use cases and also some oopsies along the way.
All the domain tests are working fine again all the Calendar/DateTime migration
A few lint complaints
Instrumented tests fixed
When renaming the module from "shared" to "domain", one property was missed. Updated!
When migrating to KMM, the checkers were removed from the Gradle script. The quality plugin was re-added and the lint fixes updated.
Now that everything is set on the correct KMM directory, no need to keep this folder anymore.
When running the Android Test task, KMP requires the minimum SDK to 30 to allow space in the test names. Another solution is to remove the spaces and use simple names. The following regex was used to make the job easier: \s+(?=(?:(?:[^`]*`){2})*[^`]*`[^`]*$)
The Instant API requires min SDK 26, so we can't use the emulator on 24 anymore
Previously, some versions were inside the Version Catalog TOML and others were in the AlkaaVersions file. Since the versioning and compiling will be even more important now that we will compile with KMP, all the versions are centralized in a single place. Also, the VersionCatalogExtensions was broken in several classes to make it easier to understand. They are: Bundles, Libraries and Versions.
Convert the Domain module to a KMP module
During the file moving, instead of creating "com/escodro/domain", I created "com.escodro/domain". Now everything is fine again.
🚚 Organize the domain folders
The Repository module was migrated from Android/Kotlin-only module to KMP. For now, we are not caring too much about ProGuard, so the files were removed.
For consistency, the class directory was named to "kotlin"
Since one more layer was converted from java.Calendar to kotlinx.DateTime, now the local layer will be responsible for the mapping.
Room was removed to make space for the new-shiny-multiplatform SQLDelight.
Replacing Room with SQLDelight was _delightful_.
igorescodro
force-pushed
the
amp/local
branch
2 times, most recently
from
August 11, 2023 14:43
55c760c
to
762bcac
Compare
A single unit test was updated
igorescodro
force-pushed
the
amp/local
branch
2 times, most recently
from
August 11, 2023 20:12
09d1197
to
e308ad6
Compare
Following the KMP module structure, the files were moved accordingly. Also some minor import changes required regarding the Dispatchers.
Oopsy, I removed the tests when moving the class between the modules. The classes are now back and removed the junit dependencies since it doesn't work well with KMM.
Removing Java's Calendar and adapting the logging tool to its multiplatform version. Some work was needed for the convertions since the Android layers will keep using Calendar for now. Future commits will address the TODOs and make it more clear.
Fix the logging system in the remaining use cases and also some oopsies along the way.
All the domain tests are working fine again all the Calendar/DateTime migration
A few lint complaints
Instrumented tests fixed
When renaming the module from "shared" to "domain", one property was missed. Updated!
When migrating to KMM, the checkers were removed from the Gradle script. The quality plugin was re-added and the lint fixes updated.
Now that everything is set on the correct KMM directory, no need to keep this folder anymore.
When running the Android Test task, KMP requires the minimum SDK to 30 to allow space in the test names. Another solution is to remove the spaces and use simple names. The following regex was used to make the job easier: \s+(?=(?:(?:[^`]*`){2})*[^`]*`[^`]*$)
The Instant API requires min SDK 26, so we can't use the emulator on 24 anymore
Previously, some versions were inside the Version Catalog TOML and others were in the AlkaaVersions file. Since the versioning and compiling will be even more important now that we will compile with KMP, all the versions are centralized in a single place. Also, the VersionCatalogExtensions was broken in several classes to make it easier to understand. They are: Bundles, Libraries and Versions.
During the file moving, instead of creating "com/escodro/domain", I created "com.escodro/domain". Now everything is fine again.
The Repository module was migrated from Android/Kotlin-only module to KMP. For now, we are not caring too much about ProGuard, so the files were removed.
For consistency, the class directory was named to "kotlin"
Since one more layer was converted from java.Calendar to kotlinx.DateTime, now the local layer will be responsible for the mapping.
igorescodro
force-pushed
the
alkaa-multiplatform
branch
from
August 14, 2023 17:24
b2c8e03
to
b148ae6
Compare
…local # Conflicts: # data/local/build.gradle.kts # data/local/src/main/java/com/escodro/local/mapper/TaskMapper.kt # features/task/src/main/java/com/escodro/task/presentation/detail/alarm/TaskAlarmViewModel.kt # features/task/src/test/java/com/escodro/task/presentation/detail/TaskAlarmViewModelTest.kt # gradle/libs.versions.toml # plugins/src/main/java/extension/CommonExtension.kt
Quick self code review there
Instrumented tests updated to work properly after all the module changes
KMP doesn't support space in names in the minimum Android SDK. Minor notification test failures fixed.
igorescodro
force-pushed
the
alkaa-multiplatform
branch
from
August 16, 2023 01:11
b148ae6
to
c8d1ff8
Compare
…local # Conflicts: # app/src/androidTest/java/com/escodro/alkaa/NotificationFlowTest.kt # data/local/build.gradle.kts # data/local/src/main/java/com/escodro/local/mapper/TaskMapper.kt # features/task/src/main/java/com/escodro/task/presentation/detail/alarm/TaskAlarmViewModel.kt # features/task/src/test/java/com/escodro/task/presentation/detail/TaskAlarmViewModelTest.kt
The TaskListFlowTest test had a typo while migrating to SDLDelight. Also a new rule was added to grant the Notification Permission for the newer SDK versions.
igorescodro
changed the title
Convert Local module to KMP
♻️ Convert Local module to KMP
Aug 28, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Convert the local module to KMP. In order to do that, we need to migrate from Room to SQLDelight.