-
Notifications
You must be signed in to change notification settings - Fork 240
Add iOS target for demo-compose-app #779
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
Add iOS target for demo-compose-app #779
Conversation
5fd0dad
to
31b0ffd
Compare
@siarhei-luskanau the CIO issue was fixed and it's available in the nightly build. I pushed the update and tada: 🎉 |
|
||
// Define keys for the preferences | ||
companion object { | ||
val OPENAI_TOKEN_KEY = stringPreferencesKey("openai_token") |
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.
nit/note: This should normakky be stored in KeyChain. I found the Multiplatform Settings library supports it. Maybe there is a more idiomatic way to support it without adding this dependency.
It's not blocker now
@siarhei-luskanau , what if we'll align the build pipeline with existing one? I'm looking forward merging this PR |
@kpavlov I'm ready to do this. Can we make a call to discuss about pipeline and expected result? |
@siarhei-luskanau My schedule is quite tight, so I can’t join a call. |
fd7fd85
to
6876c22
Compare
I have added Demo Compose app to the root Gradle project. As result demo app project can open and launched as part of root Gradle project and as independent project. Additional a have updated the checks.yml pipeline to build Demo Compose app as part of root Gradle project |
f4b3954
to
c597d63
Compare
I was wrong, I didn't managed to add Demo Compose app to the root Gradle project. |
c597d63
to
0e75ffe
Compare
@kpavlov Do you mind to keep the demo app assembling in checks.yml only and remove the demo-compose-app.yml? |
@siarhei-luskanau , maybe not, unless we a publishing koog to mavenLocal and consume it from demo app. The macos builds are pretty slow. The CI is already slow enough ... |
0e75ffe
to
ea1d6ab
Compare
@kpavlov Now he have the same work in two pipelines related the the demo app assembling. |
@siarhei-luskanau , as far as I remember, "checks" doesn't build the demo app, and demo app workflow doesn't build main project. So we should be good |
ea4ed1f
to
d9e2679
Compare
Rebased to the latest develop branch and squashed into one commit |
d9e2679
to
49ff7de
Compare
@kpavlov Updated demo app dependencies to use compose multiplatform 1.9.0.
|
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.
Thank you, @siarhei-luskanau
It works on iOS, but there is an error on Android
9d5027d
to
c28c391
Compare
@kpavlov I have updated the PR:
Jvm, Android, iOS target works as expected for me locally. |
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.
Thank you, @siarhei-luskanau 🙏🏻
Verified, works for me too
@kpavlov Please keep in mind that I have two additional PR related the demo-compose-app:
|
Motivation and Context
Breaking Changes
Type of the changes
Checklist
develop
as the base branchAdditional steps for pull requests adding a new feature