-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/ios-integration #1
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
base: main
Are you sure you want to change the base?
Conversation
785fa1d to
4a4dc8f
Compare
sdk.core/src/commonMain/kotlin/io/nyris/sdk/internal/di/ServiceModule.kt
Outdated
Show resolved
Hide resolved
smellouk
left a comment
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.
Please run detekt on your machine and fix the reported issues
FinalNewline - [File must end with a newline (\n)] at /home/runner/work/sdk-kmp/sdk-kmp/sdk.core/src/iosMain/kotlin/io/nyris/sdk/internal/util/IOSLogger.kt:1:1
FinalNewline - [File must end with a newline (\n)] at /home/runner/work/sdk-kmp/sdk-kmp/sdk.core/src/iosMain/kotlin/io/nyris/sdk/Nyris.kt:1:1
smellouk
left a comment
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.
Great work :D!!
sdk.core/src/commonMain/kotlin/io/nyris/sdk/internal/di/ServiceModule.kt
Outdated
Show resolved
Hide resolved
| class IntegrationWithKMP { | ||
| func greetings() -> Nyris { | ||
| let config = NyrisConfig(isDebug: true, baseUrl: "", httpEngine: nil, timeout: 10, platform: NyrisPlatform.ios) | ||
| return NyrisCompanion() |
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.
we could use @ObjCName(swiftName = "Nyris") to rename that from NyrisCompanion to Nyris
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.
It still doesn't solve the problem, as I need to pass a configuration object with all it's params making the actual implementation in kotlin iOS redundant.
2e72955 to
3ee92e1
Compare
smellouk
left a comment
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.
LGTM 🙏
smellouk
left a comment
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 just noticed that spotless changes are missing, can we consider to add them please
57448e5 to
ee75107
Compare
- Fix linting issues.
9b1cbca to
274739e
Compare
721d2a5 to
89d7c94
Compare
89d7c94 to
f9f4bb7
Compare
Add iOS to the KMP project