search

jdg / MBProgressHUD

MBProgressHUD + Customizations
http://www.bukovinski.com/
MIT License
16.01k stars 3.56k forks source link

Add support for SwiftPM & Accio #564

Closed Jeehut closed 4 years ago

Jeehut commented 5 years ago

This adds support for SwiftPM manifest based dependency managers. Specifically this adds support for installing via Accio but may also work with SwiftPM once it's integrated into Xcode.

Please note that this project is part of Accio's official integration tests within the Demo project.

SlashDevSlashGnoll commented 5 years ago

This becomes much more important in XCode 11 as Apple has added direct support for SPM in all xcode projects. This will soon become the preferred way to manage dependencies. Getting this in will be very helpful.

matej commented 5 years ago

Does Package.swift apply to XCode 11 as is, or would it require any changes? Anybody tried this?

mergesort commented 5 years ago

For what it's worth I tested this, and there seems to be a conflict between the module name and class being MBProgressHUD.

This is what happens when I try to extend it.

image

And issues at a call-site.

image
Jeehut commented 5 years ago

@mergesort Could the error be related to this open Swift bug?

mergesort commented 5 years ago

It quite likely could be. I'm wondering what a good fix would be that adds SPM support. All changes seem like they would be breaking, but I assume the easiest would be to change the module name rather than all of the classes?

Jeehut commented 5 years ago

I agree, changing the module name within the Package.swift file would probably be the least affecting method of getting it working with SwiftPM. We would probably need to specify the path for the target explicitly and document that the import statement for SwiftPM users would be different, maybe just MBProgress?

mergesort commented 5 years ago

That sounds like a good suggestion to me, and after a few days I can't really think of a better way to resolve the conflict.

Schaefers commented 5 years ago

Can you perform these changes on your fork? Would be happy to see this merged or use your fork.

4brunu commented 4 years ago

Hey, for those interested, I just created https://github.com/jdg/MBProgressHUD/pull/592 that doesn't have the conflict between the module name and class being MBProgressHUD. If @Jeehut wants to incorporate the changes in this PR or just merge the other PR, I'm fine with both options 🙂

Jeehut commented 4 years ago

Thank you @4brunu, I just rebased my fork, cherry-picked your changes in a single commit and added a badge that MBProgressHUD supports SwiftPM to the README. I think you can close #592 now ad a duplicate to this. Great work with the modulemap etc.!

4brunu commented 4 years ago

@Jeehut no problem 🙂 Done, I closed the PR. What do you think about adding the SPM instructions to the readme? I saw the Accio instructions but not the SPM. I know they are almost the same, but it's just to clarify to the users when they look for SPM in the readme.

Jeehut commented 4 years ago

@4brunu Makes sense. It's just the same steps, only the command is different, fixed it! 👍

4brunu commented 4 years ago

Thanks 👍 I have tested this PR locally and it's working, for me it's ready to merge 🙂

matej commented 4 years ago

Alright, let's give this a try. I don't use Swift package manager, so I'll just rely on your expertise here. Thank you for your contribution!

4brunu commented 4 years ago

Nice, thanks 👍 Could you please also make a new release? Because it would be nicer to point SPM to a tag instead of a branch or a commit 🙂