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.

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

[#8] [KMM] [Backend] Set Up Network layer Core with Ktor #51

Merged
merged 16 commits into from
Oct 27, 2022

Conversation

blyscuit
Copy link
Owner

@blyscuit blyscuit commented Sep 20, 2022

close #8

What Happened

Insight

  • Move dependencies to buildsrc files.
  • Use new memory manager for iOS.
  • BuildKonfig is generated from CICD.
  • Use a special github.com/nimblehq/jsonapi-kotlin as submodule.

Proof of Work

Screen Shot 2022-09-20 at 15 33 21

Screen Shot 2022-09-21 at 09 58 42

@blyscuit blyscuit self-assigned this Sep 20, 2022
@blyscuit blyscuit force-pushed the feature/#8-set-up-network-layer branch 5 times, most recently from c200409 to 527870e Compare September 20, 2022 10:10
@github-actions
Copy link

github-actions bot commented Sep 20, 2022

11 Warnings
⚠️ Big PR
⚠️ Please remove period from end of commit subject line.
82a5165
⚠️ Please remove period from end of commit subject line.
b05e2a5
⚠️ Uh oh! Koin.kt is under 95% coverage!
⚠️ Uh oh! NetworkClient.kt is under 95% coverage!
⚠️ Uh oh! NetworkMockModel.kt is under 95% coverage!
⚠️ Uh oh! NetworkModule.kt is under 95% coverage!
⚠️ Uh oh! RepositoryModule.kt is under 95% coverage!
⚠️ Uh oh! Start.kt is under 95% coverage!
⚠️ Uh oh! UseCaseModule.kt is under 95% coverage!
⚠️ Uh oh! Your project is under 80% coverage!

🧛 shared Code Coverage: 73.89%

Coverage of Modified Files:

File Coverage
JsonMockEngine.kt 100.00%
Koin.kt 0.00%
NetworkClient.kt 76.83%
NetworkClientTest.kt 100.00%
NetworkMockModel.kt 86.61%
NetworkModule.kt 0.00%
RepositoryModule.kt 0.00%
Start.kt 0.00%
UseCaseModule.kt 0.00%

Modified Files Not Found In Coverage Report:

.gitignore
.gitmodules
AndroidManifest.xml
BuildKonfig.kt.example
Dangerfile
Dependency.kt
KoinApplication.swift
LazyKoin.swift
Module.kt
Plugin.kt
Podfile
Podfile.lock
Typealias+Koin.swift
Versions.kt
build.gradle.kts
build.gradle.kts
deploy_ios_firebase.yml
deploy_ios_release_firebase.yml
gradle.properties
ios_review_pull_request.yml
kmm_review_pull_request.yml
nimble-jsonapi-kotlin
project.pbxproj
settings.gradle.kts

Codebase cunningly covered by count Shroud 🧛

Generated by 🚫 Danger

@github-actions
Copy link

github-actions bot commented Sep 20, 2022

Current coverage for Survey is 58.05%

Files changed - -
LazyKoin.swift 0.00% 💀
KoinApplication.swift 0.00% 💀

Powered by xcov

Generated by 🚫 Danger

@blyscuit blyscuit force-pushed the feature/#8-set-up-network-layer branch from f60eee4 to 078d5d5 Compare September 21, 2022 10:10
@blyscuit blyscuit force-pushed the feature/#8-set-up-network-layer branch 8 times, most recently from f818f06 to e71863f Compare September 22, 2022 08:22
@blyscuit blyscuit marked this pull request as ready for review September 22, 2022 09:49
@blyscuit blyscuit force-pushed the feature/#8-set-up-network-layer branch from d1d0b55 to 0d404e8 Compare September 27, 2022 08:38
@blyscuit blyscuit requested a review from minhnimble September 27, 2022 09:44
Copy link
Collaborator

@minhnimble minhnimble left a comment

Choose a reason for hiding this comment

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

Anyhow, LGTM now 🚀

@blyscuit blyscuit requested a review from luongvo September 29, 2022 05:34
Copy link
Collaborator

@luongvo luongvo left a comment

Choose a reason for hiding this comment

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

rest lgtm

.github/workflows/deploy_ios_release_firebase.yml Outdated Show resolved Hide resolved
.github/workflows/deploy_ios_firebase.yml Outdated Show resolved Hide resolved
.github/workflows/deploy_ios_release_firebase.yml Outdated Show resolved Hide resolved
.github/workflows/kmm_review_pull_request.yml Outdated Show resolved Hide resolved
.gitmodules Show resolved Hide resolved
buildSrc/src/main/kotlin/myPackage/Modules.kt Outdated Show resolved Hide resolved
shared/build.gradle.kts Show resolved Hide resolved
shared/build.gradle.kts Outdated Show resolved Hide resolved
@blyscuit blyscuit requested a review from luongvo October 18, 2022 09:37
@blyscuit blyscuit force-pushed the feature/#8-set-up-network-layer branch from ed98f64 to b05e2a5 Compare October 19, 2022 06:05
Copy link
Collaborator

@luongvo luongvo left a comment

Choose a reason for hiding this comment

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

@blyscuit 1 issue left, rest lgtm

Copy link
Collaborator

@luongvo luongvo left a comment

Choose a reason for hiding this comment

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

lgtm

@blyscuit blyscuit merged commit 2e3aee7 into develop Oct 27, 2022
@blyscuit blyscuit deleted the feature/#8-set-up-network-layer branch October 27, 2022 04:45
Comment on lines +124 to +179
buildkonfig {
packageName = "co.nimblehq.blisskmmic"

defaultConfigs {
buildConfigField(
STRING,
"CLIENT_ID",
BuildKonfig.CLIENT_ID
)
buildConfigField(
STRING,
"CLIENT_SECRET",
BuildKonfig.CLIENT_SECRET
)
buildConfigField(
STRING,
"BASE_URL",
BuildKonfig.STAGING_BASE_URL
)
}

defaultConfigs("production") {
buildConfigField(
STRING,
"CLIENT_ID",
BuildKonfig.CLIENT_ID
)
buildConfigField(
STRING,
"CLIENT_SECRET",
BuildKonfig.CLIENT_SECRET
)
buildConfigField(
STRING,
"BASE_URL",
BuildKonfig.PRODUCTION_BASE_URL
)
}

defaultConfigs("staging") {
buildConfigField(
STRING,
"CLIENT_ID",
BuildKonfig.CLIENT_ID_STAGING
)
buildConfigField(
STRING,
"CLIENT_SECRET",
BuildKonfig.CLIENT_SECRET_STAGING
)
buildConfigField(
STRING,
"BASE_URL",
BuildKonfig.STAGING_BASE_URL
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@blyscuit Sorry for recalling you on a merged PR, I think there is still something wrong here. We need to define a properties file for BuildKonfig and read them from the properties file instead of a BuildKonfig class. The logic here is BuildKonfig lib helps us to create BuildKonfig class file to use in runtime from a properties config, so BuildKonfig class is the result, not the source of this gradle config.

Comment on lines +1 to +10
object BuildKonfig {
const val CLIENT_ID = ""
const val CLIENT_SECRET = ""
const val STAGING_BASE_URL = ""
const val PRODUCTION_BASE_URL = ""
const val CLIENT_ID_STAGING = ""
const val CLIENT_SECRET_STAGING = ""
const val UI_TEST_EMAIL = ""
const val UI_TEST_PASSWORD = ""
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This sample file is incorrect. It should be a properties file when working with Gradle build system 💭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants