Skip to content

Added starting point to move over to cocoapods plugin #349

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

Merged
merged 25 commits into from
Apr 3, 2023

Conversation

Reedyuk
Copy link
Collaborator

@Reedyuk Reedyuk commented Jan 27, 2023

Kotlin 1.8.20 has a few fixes that allows us to use cocoapods instead of carthage.
Converted all subprojects to use cocoapods plugin instead of carthage.
Added caching(works better when its in main branch)

@nbransby or @michaelprichardson - do you want to review this? its a fairly big change.

@Reedyuk Reedyuk marked this pull request as ready for review April 3, 2023 12:50
@nbransby
Copy link
Member

nbransby commented Apr 3, 2023

LGTM - what is the recommended way to include the firebase-kotlin-sdk and firebase in your xcode project now then?

@Reedyuk
Copy link
Collaborator Author

Reedyuk commented Apr 3, 2023

Two ways, either generating the pod file or you could just add like we normally do.
The difference now should be the kmp projects that use this library, you shouldn't have to add the dependency for compilation - there is several issues in GitHub raised around this, framework not found

@Reedyuk Reedyuk merged commit 30cc1f9 into master Apr 3, 2023
@Reedyuk Reedyuk deleted the cocoapods-update branch April 3, 2023 19:17
@walkingbrad
Copy link
Contributor

walkingbrad commented Apr 20, 2023

Umm... so all of iOS is now TODO? Is there a plan for that? It was very surprising to spend the time updating to the latest release here and find that everything was broken at runtime for iOS.

@Reedyuk
Copy link
Collaborator Author

Reedyuk commented Apr 21, 2023

Umm... so all of iOS is now TODO? Is there a plan for that? It was very surprising to spend the time updating to the latest release here and find that everything was broken at runtime for iOS.

What do you mean everything is todo? It's working. What isn't working for you?

@walkingbrad
Copy link
Contributor

@Reedyuk Oh I guess it's only for creating Firebase product classes from a specific FirebaseApp instance (which I use extensively in my app). I refer to all of these: TODO("Come back to issue")

Still a very sneaky and surprising change that would've been nice to have more upfront communication about in release notes or even the PR description.

So what was the issue there? Something very tricky? Happy to assist if if it's not :P

@Reedyuk
Copy link
Collaborator Author

Reedyuk commented Apr 21, 2023

I've not had chance to look at it, yes sorry was my fault.
Happy for you to put a pr in for the changes. I think it was a slight change to the api in iOS.

@Reedyuk
Copy link
Collaborator Author

Reedyuk commented Apr 21, 2023

A lot of time was spent just getting it to work with cocoa pods plug-in and then the pain of compilation 😂

@walkingbrad
Copy link
Contributor

Are you suggesting that bridging Gradle, Kotlin, iOS, and Cocoapods is complicated? 😂 😂 😂

I'll take a dive into it next week maybe.

@Reedyuk
Copy link
Collaborator Author

Reedyuk commented Apr 22, 2023

Are you suggesting that bridging Gradle, Kotlin, iOS, and Cocoapods is complicated? 😂 😂 😂

I'll take a dive into it next week maybe.

It seems like there is an issue with the ios commoizer.

Screenshot 2023-04-22 at 08 51 08

It might an issue i need to raise with the jetbrains team.

@walkingbrad
Copy link
Contributor

Could this fix it or be related? I noticed a warning in my own project that recommended I add this after upgrading to kotlin 1.8.20

kotlin.mpp.enableCInteropCommonization=true in gradle settings

https://kotlinlang.org/docs/multiplatform-share-on-platforms.html#share-code-on-all-platforms

@walkingbrad
Copy link
Contributor

@Reedyuk I've got a PR here #379 if you want to take a look.

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.

3 participants