Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[12] Integrate Moko Resources to share project resources between platforms #44

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from
Draft

[12] Integrate Moko Resources to share project resources between platforms #44

wants to merge 6 commits into from

Conversation

AVI5HEK
Copy link

@AVI5HEK AVI5HEK commented Sep 8, 2023 •
edited
Loading

What happened 👀

Add Moko Resources to the template.

Insight 📝

  • Added Moko Resources according to the instructions on the official GitHub page and with the help of this video.
  • For some reason, the :shared module fails to build if we add the dependency dev.icerock.moko:resources-compose:0.23.0. This line has been commented out for now until a resolution is found.
  • Added expect and actual classes and methods of Strings as a helper to get the strings from Moko resources in both the Android and iOS apps.

Proof Of Work 📹

Android

iOS

image

@AVI5HEK AVI5HEK added this to the 0.2.0 milestone Sep 8, 2023
@AVI5HEK AVI5HEK self-assigned this Sep 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 8, 2023 •
edited
Loading

10 Warnings
⚠️ Big PR
⚠️ Uh oh! HomeScreen.kt is under 95% coverage!
⚠️ Uh oh! Strings.kt is under 95% coverage!
⚠️ Uh oh! Your project is under 80% coverage!
⚠️ shared/src/androidMain/kotlin/co/nimblehq/kmm/template/Platform.android.kt#L3 - The file name 'Platform.android' does not match the name of the single top-level declaration 'AndroidPlatform'.
⚠️ shared/src/androidUnitTest/kotlin/co/nimblehq/kmm/template/androidTest.kt#L6 - The file name 'androidTest' does not match the name of the single top-level declaration 'AndroidGreetingTest'.
⚠️ shared/src/commonTest/kotlin/co/nimblehq/kmm/template/commonTest.kt#L6 - The file name 'commonTest' does not match the name of the single top-level declaration 'CommonGreetingTest'.
⚠️ shared/src/commonTest/kotlin/co/nimblehq/kmm/template/extensions/ResponseMappingTest.kt#L18 - Exception is a too generic Exception. Prefer throwing specific exceptions that indicate a specific error case.
⚠️ shared/src/iosMain/kotlin/co/nimblehq/kmm/template/Platform.ios.kt#L5 - The file name 'Platform.ios' does not match the name of the single top-level declaration 'IOSPlatform'.
⚠️ shared/src/iosTest/kotlin/co/nimblehq/kmm/template/iosTest.kt#L6 - The file name 'iosTest' does not match the name of the single top-level declaration 'IosGreetingTest'.

Kover report:

🧛 Unit Tests Code Coverage: 10.31%

Coverage of Modified Files:

File Coverage
HomeScreen.kt 0.00%
Strings.kt 0.00%

Modified Files Not Found In Coverage Report:

Dependencies.kt
Dependencies.kt
Gemfile.lock
Plugins.kt
Plugins.kt
Podfile.lock
Versions.kt
Versions.kt
build.gradle.kts
build.gradle.kts
build.gradle.kts
build.gradle.kts
project.pbxproj
strings.xml
strings.xml

Codebase cunningly covered by count Shroud 🧛

Generated by 🚫 Danger

@AVI5HEK AVI5HEK force-pushed the feature/12-moko-resources branch 4 times, most recently from 733215b to 9d544fd Compare September 14, 2023 10:50
@AVI5HEK AVI5HEK changed the base branch from develop to feature/29-reuse-android-module-from-android-templates September 14, 2023 11:06
@AVI5HEK AVI5HEK force-pushed the feature/12-moko-resources branch 2 times, most recently from 9828b2b to 50865c4 Compare September 14, 2023 11:44
@AVI5HEK AVI5HEK changed the base branch from feature/29-reuse-android-module-from-android-templates to develop September 14, 2023 11:44
github-actions[bot]
github-actions bot reviewed Sep 14, 2023
View reviewed changes
shared/src/androidMain/kotlin/co/nimblehq/kmm/template/Strings.kt
return if (args.isEmpty()) {
StringDesc.Resource(id).toString(context = context)
} else {
id.format(*args.toTypedArray()).toString(context)
Copy link

@github-actions github-actions bot Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty.

github-actions[bot]
github-actions bot reviewed Sep 14, 2023
View reviewed changes
shared/src/iosMain/kotlin/co/nimblehq/kmm/template/Strings.kt
return if (args.isEmpty()) {
StringDesc.Resource(id).localized()
} else {
id.format(*args.toTypedArray()).localized()
Copy link

@github-actions github-actions bot Sep 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ In most cases using a spread operator causes a full copy of the array to be created before calling a method. This may result in a performance penalty.

@AVI5HEK AVI5HEK marked this pull request as ready for review September 14, 2023 12:40
@github-actions
Copy link

github-actions bot commented Sep 14, 2023 •
edited
Loading

1 Warning
⚠️ This pull request is quite big (554 lines changed), please consider splitting it into multiple pull requests.

Current coverage for sample is 18.25%

✅ No files affecting coverage found

Powered by xcov

Generated by 🚫 Danger

@luongvo luongvo changed the base branch from develop to feature/29-reuse-android-module-from-android-templates September 19, 2023 02:12
@luongvo
Copy link
Member

luongvo commented Sep 19, 2023

@AVI5HEK Please base this PR on #32 to reuse the Android app module from Android Templates, then try a way to inject your new changes for the KMM Templates 🙏

kaungkhantsoe
kaungkhantsoe reviewed Sep 28, 2023
View reviewed changes
sample/shared/build.gradle.kts Outdated
@@ -59,6 +62,11 @@ kotlin {
implementation(AUTH)
}

with(Dependencies.Moko) {
api(RESOURCES)
// api(RESOURCES_COMPOSE) FIXME: Cannot build the shared module with this dependency
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what is the issue with this dependency but I can build the project with this line @AVI5HEK. Can you please provide the logs of the error?

Screen.Recording.2023-09-28.at.8.39.15.AM.mov

Base automatically changed from feature/29-reuse-android-module-from-android-templates to develop October 2, 2023 07:12
@AVI5HEK AVI5HEK marked this pull request as draft October 31, 2023 09:39
@luongvo luongvo modified the milestones: 0.2.0, 0.3.0 Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@kaungkhantsoe kaungkhantsoe kaungkhantsoe left review comments

@suho suho Awaiting requested review from suho suho is a code owner

@luongvo luongvo Awaiting requested review from luongvo luongvo is a code owner

@phongvhd93 phongvhd93 Awaiting requested review from phongvhd93 phongvhd93 is a code owner

@markgravity markgravity Awaiting requested review from markgravity markgravity is a code owner

@lydiasama lydiasama Awaiting requested review from lydiasama

At least 2 approving reviews are required to merge this pull request.

Assignees

@AVI5HEK AVI5HEK

Labels
type : feature
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.

Integrate Moko Resources to share project resources between platforms
3 participants
@AVI5HEK @luongvo @kaungkhantsoe