-
Notifications
You must be signed in to change notification settings - Fork 189
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
base: master
Are you sure you want to change the base?
Migrate to SPM #116
Conversation
@pmusolino what do you think? |
It would be great to see this merged @pmusolino 🙌 |
Could you please merge that? |
Please merge this when possible, this will be of real help |
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! |
@pmusolino the only diff I see is that the files moved to a different directory |
@dnKaratzas ActionableTableViewCell.xib was not moved but has some changes for example. |
@pmusolino I don't recall making changes to |
@pmusolino minor changes cause opened the |
Right. I agree with @dnKaratzas & @ipavlidakis as well Can you please merge it? @pmusolino Thank you in advance.. |
It would be great to see this merged @pmusolino , Any updates? |
Hello @pmusolino, Can you please merge this PR? cc: @ipavlidakis Thanks. |
We really need this. |
@ipavlidakis @pmusolino Any updates? |
@ipavlidakis could I ask you to fix the merge conflicts, so I can review this PR and merge it? |
@ipavlidakis do you have time to fix the merge conflicts? It would be really helpful to offer SPM 🙌 thanks! |
Sure, i'm on it and should have something ready in following few hours 🤞🏻 |
@ipavlidakis do you have any update on this? 🙏 |
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 |
Thanks @ipavlidakis! Can I test it right away or should you need to apply extra changes? |
Feel free to test it as it is. |
I'll test it this weekend and early next week, thanks @ipavlidakis 🙏 |
@ipavlidakis I'm testing the PR. The first issue that I noticed, is that while I try to run 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. Any suggestion on how to move forward? |
Hello @ipavlidakis , PR: ipavlidakis#3 Below line should be removed, There is no similar file. And, |
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!! |
@ipavlidakis let us know when you will be able to take care of this, thanks! 🙌 |
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. |
Thank you @ipavlidakis for finding the time! I did a test with Cocoapods, and I got an error in this file: While, commenting it, allowed me to compile the app and launch WormHoly, and everything seems to work properly. |
Thanks for checking @pmusolino. I just updated podspec to ignore the file. do you mind trying one more time? |
The issue still persists @ipavlidakis, it seems the file has not been excluded, I'm still able to see it. |
Interesting, maybe the pattern was wrong. I simplified it to be just the folder. Hope it works now |
@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. 👏 |
@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? |
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 |
@pmusolino do you have some code i can look into? |
@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? |
@ipavlidakis do you have any news on this? |
Hello Team, Can you please merge & release latest changes? @ipavlidakis We are waiting for this changes to be merge from your side. |
Hi! We are waiting too. It would be cool to have it. Tx! |
Hello @ipavlidakis, Can you merge changes? Thanks! |
@ipavlidakis 🙌 do you have time to double-check the issues mentioned before? |
@pmusolino : did we, by any chance got the time to merge this branch for SPM support for Wormholy |
Still not merged? Can you please merge it if everything works fine? @pmusolino |
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. |
Had to remove this dependency in favor of SPM, sad because this extension is great |
I am also waiting for new release version of sdk |
Hello @pmusolino , Can you please take a look into updated PR? & merge it if all good. cc: @ipavlidakis |
Resolves #112, #139