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

Migrate to SPM #116

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Migrate to SPM #116

wants to merge 2 commits into from

Conversation

ipavlidakis
Copy link

@ipavlidakis ipavlidakis commented May 9, 2022

  • Migrated to SPM
  • Introduced WormholySwift as a wrapper target for Swift files as SPM doesn't support multi-lang modules
  • Cleaned up and update demo
  • Removed configs as unnecessary
  • Updated podfile

Resolves #112, #139

@ipavlidakis
Copy link
Author

@pmusolino what do you think?

@gmoraleda
Copy link

It would be great to see this merged @pmusolino 🙌
Thanks for the PR @ipavlidakis !

@dioKaratzas
Copy link

Could you please merge that?

@zdurlan
Copy link

zdurlan commented Dec 29, 2022

Please merge this when possible, this will be of real help

@pmusolino
Copy link
Owner

Hey @ipavlidakis, thank you so much for your contribution! I was just wondering if you could help me understand the changes you made in this PR that aren't related to the SPM migration, such as the changes to the XIB files. Would you be able to explain the reasoning behind those changes? Thanks again!

@dioKaratzas
Copy link

@pmusolino the only diff I see is that the files moved to a different directory

@pmusolino
Copy link
Owner

@dnKaratzas ActionableTableViewCell.xib was not moved but has some changes for example.

@ipavlidakis
Copy link
Author

@pmusolino I don't recall making changes to .xib files other than moving them to match the SPM file structure. The changes were probably made from the different version of Xcode, that's why you can see mainly changes on version fields

@dioKaratzas
Copy link

dioKaratzas commented Mar 7, 2023

@pmusolino minor changes cause opened the xib on a different Xcode version. So those are Xcode changes.
Then onlly actual change I can spot is the module name that is an accepted change:
customModule="WormholySwift"

@mparmar-covantex
Copy link

Right. I agree with @dnKaratzas & @ipavlidakis as well

Can you please merge it? @pmusolino

Thank you in advance..

@mparmar-covantex
Copy link

It would be great to see this merged @pmusolino , Any updates?

@mehulparmar4ever
Copy link

Hello @pmusolino, Can you please merge this PR? cc: @ipavlidakis

Thanks.

@azdurlan-spectra
Copy link

We really need this.

@mehulparmar4ever
Copy link

@ipavlidakis @pmusolino Any updates?

@pmusolino
Copy link
Owner

@ipavlidakis could I ask you to fix the merge conflicts, so I can review this PR and merge it?

@ipavlidakis ipavlidakis reopened this Feb 28, 2024
@pmusolino
Copy link
Owner

@ipavlidakis do you have time to fix the merge conflicts? It would be really helpful to offer SPM 🙌 thanks!

@ipavlidakis
Copy link
Author

Sure, i'm on it and should have something ready in following few hours 🤞🏻

@pmusolino
Copy link
Owner

@ipavlidakis do you have any update on this? 🙏

@pmusolino pmusolino added the enhancement New feature or request label Mar 18, 2024
@ipavlidakis
Copy link
Author

Hey @pmusolino sorry for the delay. I have updated the PR and I'm going to test it tomorrow. It will be great if you can also check it and let me know your thoughts. Thanks

@pmusolino
Copy link
Owner

Thanks @ipavlidakis! Can I test it right away or should you need to apply extra changes?

@ipavlidakis
Copy link
Author

Feel free to test it as it is.

@pmusolino
Copy link
Owner

pmusolino commented Mar 22, 2024

I'll test it this weekend and early next week, thanks @ipavlidakis 🙏

@pmusolino
Copy link
Owner

@ipavlidakis I'm testing the PR.

The first issue that I noticed, is that while I try to run WormholyDemo on the simulator, I get this error: Missing package product Wormholy

Screenshot 2024-03-25 at 14 37 11

Taking a look at the structure of the Xcode project, it seems that all the classes of the library, the folders, etc... are missing. I'm using Xcode 15.2.0.

Screenshot 2024-03-25 at 14 43 11

Any suggestion on how to move forward?

@parmar-mehul
Copy link

parmar-mehul commented Mar 26, 2024

Hello @ipavlidakis ,

PR: ipavlidakis#3

Below line should be removed, There is no similar file.
"Support Files/Assets.xcassets"

Screenshot 2024-03-26 at 9 43 51 AM

And,
You can delete this two files as well, May be, That is old structure. Remove it and Build demo app, It works.

Screenshot 2024-03-26 at 9 40 17 AM

Screenshot 2024-03-26 at 9 40 33 AM

@pmusolino
Copy link
Owner

Thanks, @parmar-mehul for the collaboration 🚀 let's wait for @ipavlidakis to review and merge your PR.

@parmar-mehul
Copy link

Thanks, @parmar-mehul for the collaboration 🚀 let's wait for @ipavlidakis to review and merge your PR.

Hello @ipavlidakis, Do we have any updates? It has been a week now! Hope we will get this done soon. Thank you for your time!!

@pmusolino
Copy link
Owner

@ipavlidakis let us know when you will be able to take care of this, thanks! 🙌

@ipavlidakis
Copy link
Author

Sorry for the wait folks but that ended up being much more complicated than it was when the PR was initially opened (that combined with limited personal time to work on it made me pick up on it late).

I tested the current changes with a project integrating the library via SPM and made sure that embedded Demo project builds. It would be great if any of you is available to do a quick check with Cocoapods & Carthage as I haven't used them in a very long time and it may take me some time to set them up.

@pmusolino let me know your thoughts.

@pmusolino
Copy link
Owner

Thank you @ipavlidakis for finding the time!

I did a test with Cocoapods, and I got an error in this file:

Screenshot 2024-04-18 at 17 15 52

While, commenting it, allowed me to compile the app and launch WormHoly, and everything seems to work properly.

@ipavlidakis
Copy link
Author

Thanks for checking @pmusolino. I just updated podspec to ignore the file. do you mind trying one more time?

@pmusolino
Copy link
Owner

The issue still persists @ipavlidakis, it seems the file has not been excluded, I'm still able to see it.

@ipavlidakis
Copy link
Author

Interesting, maybe the pattern was wrong. I simplified it to be just the folder. Hope it works now

@pmusolino
Copy link
Owner

@ipavlidakis Now it works like a charm. I also tried to run the demo, and it works well! I'll review the code ASAP, and we can merge it. 👏

@pmusolino
Copy link
Owner

@ipavlidakis I started testing it with SPM. It has been integrated correctly into a project, with no issues. But Wormholy cannot be presented: manual fire or shake doesn't work. Any idea?

@federicosacca
Copy link

Hi @pmusolino, the problem is that the class at line 7 in the file (1) is 0x0 (null), because the correct class seems to be _WormholySwift. Maybe there is an issue in the package.swift declaration. Even if you change the class from @Wormholy.Wormholy to @Wormholy._WormholySwift, you will have problem in Flow.storyboard instantation, file (2).

(1) https://github.com/pmusolino/Wormholy/blob/master/Sources/Objc/WormholyConstructor.m
(2) https://github.com/pmusolino/Wormholy/blob/master/Sources/UI/Flow.storyboard

@ipavlidakis
Copy link
Author

@pmusolino do you have some code i can look into?

@pmusolino
Copy link
Owner

@ipavlidakis no code I can share. In any case, it's a project with both CocoaPods and SPM. Do you have the time to check what @federicosacca mentioned above?

@pmusolino
Copy link
Owner

@ipavlidakis do you have any news on this?

@parmar-mehul
Copy link

parmar-mehul commented Jun 17, 2024

Hello Team, Can you please merge & release latest changes? @ipavlidakis

We are waiting for this changes to be merge from your side.

@acamcor-seatcode
Copy link

Hi! We are waiting too. It would be cool to have it. Tx!

@parmar-mehul
Copy link

parmar-mehul commented Jul 8, 2024

Hello @ipavlidakis, Can you merge changes? Thanks!

@pmusolino
Copy link
Owner

@ipavlidakis 🙌 do you have time to double-check the issues mentioned before?

@Abhishek2383
Copy link

@pmusolino : did we, by any chance got the time to merge this branch for SPM support for Wormholy

@Silviuvr
Copy link

Still not merged? Can you please merge it if everything works fine? @pmusolino

@pmusolino
Copy link
Owner

Hi there, @Abhishek2383 and @Silviuvr. Unfortunately, this still doesn't work as expected. Additionally, I believe it is not possible to target only debugging or only production in Swift Package Manager, so it will be your responsibility to manage this condition on your apps.

@julian-zatloukal
Copy link

Had to remove this dependency in favor of SPM, sad because this extension is great

@mehulparmar4ever
Copy link

I am also waiting for new release version of sdk

@parmar-mehul
Copy link

Hello @pmusolino ,

Can you please take a look into updated PR? & merge it if all good. cc: @ipavlidakis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swift Package Manager SPM support?