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

Feature/ios-integration #1

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

AnasMostefaoui
Copy link
Collaborator

@AnasMostefaoui AnasMostefaoui commented Mar 19, 2023

Add iOS to the KMP project

@AnasMostefaoui AnasMostefaoui force-pushed the feature/ios-integration branch from 785fa1d to 4a4dc8f Compare March 19, 2023 14:15
Copy link
Collaborator

@smellouk smellouk left a 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

gradle.properties Outdated Show resolved Hide resolved
Copy link
Collaborator

@smellouk smellouk left a 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/build.gradle.kts Show resolved Hide resolved
sdk.ui/iOS/NyrisSearcher.xcodeproj/project.pbxproj 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()
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

@AnasMostefaoui AnasMostefaoui force-pushed the feature/ios-integration branch from 2e72955 to 3ee92e1 Compare March 24, 2023 10:40
Copy link
Collaborator

@smellouk smellouk left a comment

Choose a reason for hiding this comment

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

LGTM 🙏

Copy link
Collaborator

@smellouk smellouk left a 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

smellouk
smellouk previously approved these changes Apr 2, 2023
@smellouk smellouk force-pushed the feature/ios-integration branch 4 times, most recently from 9b1cbca to 274739e Compare April 6, 2023 09:03
@smellouk smellouk force-pushed the feature/ios-integration branch 2 times, most recently from 721d2a5 to 89d7c94 Compare April 6, 2023 09:49
@smellouk smellouk force-pushed the feature/ios-integration branch from 89d7c94 to f9f4bb7 Compare April 6, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants