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

Added Koin Dependency #31

Merged
merged 8 commits into from
Nov 12, 2024
Merged

Conversation

ghost
Copy link

@ghost ghost commented Nov 8, 2024

Added koin dependency under presentation and domain logic.

Copy link
Collaborator

@rodvar rodvar left a comment

Choose a reason for hiding this comment

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

hey @nis-ship-it thanks for your first contribution!

I've left a comment on the code to review with you.

Also, it would be great if you could also add an example of usage at least of the new libs to be used.

You could for example check how the presenters are being injected in the different main views of each app (e.g. androidClient MainActivity) and replace the current hardcoded instanciation of the depedencies (repository, presenter, etc) you could replace it with the Koin DI

That would help a lot for other devs to copy and start using Koin.
Let me know you thoughts!

bisqapps/shared/presentation/build.gradle.kts Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Nov 9, 2024

@rodvar I don't know much on ios side like if we can really call kt code? In this case koin

@rodvar
Copy link
Collaborator

rodvar commented Nov 10, 2024

@rodvar I don't know much on ios side like if we can really call kt code? In this case koin

thanks @nis-ship-it I'll have a look into this soon !

@rodvar
Copy link
Collaborator

rodvar commented Nov 11, 2024

@nis-ship-it had the first look and its looking good, some comments here:

  1. for the naming in iOS (DomainGreetingRepository) -> this is because of the .h file iOS-generated by KMP, it's including the module when you are referring from one module (presentation) to another (domain). Can we investigate using typealiases ? Also there is an experimental @ObjCName I tested but couldn't get it to work. We could also accept this different naming in iOS as you did, there shouldn't be much code in swift anyways.
  2. the androidNode implementation is missing (it would be something identical to what you've done with the androidClient I believe.

@ghost
Copy link
Author

ghost commented Nov 11, 2024

I tried the aliases but it's the same error so dnt knw about that it would work. Forgot about the node side, I will add koin to it

@rodvar rodvar force-pushed the feature/koinInjection branch from a15ee91 to ecac27e Compare November 12, 2024 02:29
@rodvar
Copy link
Collaborator

rodvar commented Nov 12, 2024

rebased branch to latest main changes

Copy link
Collaborator

@rodvar rodvar left a comment

Choose a reason for hiding this comment

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

This is pretty much ready to be merge, just some nits in terms of code formatting / unnecessary changes to be reviewed

well done @nis-ship-it !

@rodvar rodvar merged commit 3de24c6 into bisq-network:main Nov 12, 2024
1 check passed
@rodvar
Copy link
Collaborator

rodvar commented Nov 12, 2024

Thanks @nis-ship-it for your first contribution to bisq-mobile! 🎉

Please reach out on matrix if you would like to continue contributing 💪

rodvar pushed a commit to rodvar/bisq-mobile that referenced this pull request Nov 12, 2024
* Added Koin Dependency

At this stage, have only added in build.gradle.kts file under comman
code.

* Added sample code on android side

* Added di under shared package

* Added Koin under ios

* Fix - Renaming files

* Added Koin to Android Node

* Fix - Reformat the code

* Fix - fomatting again
rodvar pushed a commit to rodvar/bisq-mobile that referenced this pull request Nov 13, 2024
* Added Koin Dependency

At this stage, have only added in build.gradle.kts file under comman
code.

* Added sample code on android side

* Added di under shared package

* Added Koin under ios

* Fix - Renaming files

* Added Koin to Android Node

* Fix - Reformat the code

* Fix - fomatting again
rodvar added a commit that referenced this pull request Nov 13, 2024
* Remove protobuf files and i18n resources

* Remove mobile package (was first dev app with showing version number)

* Add multidex dependency, add protobuf source path

* - small adaptations on android native POC whilst understanding it

* - convert android native POC code to Kotlin for easier transition

* - replace completable future with coroutines job

* - protobuf fixes on POC merged from boilingfrog, fully communicates with seednode now (thanks Henrik!)

* - integration of last commit into the androidNode

* - integrated services from POC with all their dependencies, ready to be used

* - initialize services, added typesafe conf where its expecting it

* - integration of print default sec key and language, printing on system.out

* Added Koin Dependency (#31)

* Added Koin Dependency

At this stage, have only added in build.gradle.kts file under comman
code.

* Added sample code on android side

* Added di under shared package

* Added Koin under ios

* Fix - Renaming files

* Added Koin to Android Node

* Fix - Reformat the code

* Fix - fomatting again

* - integrated services from POC with all their dependencies, ready to be used

* - hardcode presenter dependency till issues with DI Koin and androidNode gets fixed

* - remove protobuf lite dep causing trouble with protobuf in bisq2 jars on runtime

* - capable of creating profile if non existent and gets persisted

* - observe network changes and log, fetch and print btc market price

* - private messaging (trading) integration

* - public messaging integration

* - Implementation for android node memory report integrated with main service + junit setup and test for the new code

* - android node ci config to use its own presenter as main

* - integration of app state

* - add cleanup on presenter destruction

* - fix userProfile mode package name

---------

Co-authored-by: HenrikJannsen <[email protected]>
Co-authored-by: nis-ship-it <[email protected]>
@ghost ghost deleted the feature/koinInjection branch November 13, 2024 05:37
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.

1 participant