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

build(spm): add swift package manifest #21

Merged
merged 4 commits into from Feb 12, 2020
Merged

Conversation

sherwinski
Copy link
Contributor

This PR introduces the addition of a Package.swift manifest, which will allow users to import imgix-swift via the Swift Package Manager. As far as I know, there's no way to test this actually works without the file being checked into the remote repository so I am hoping to get at least one set of experienced eyes on it first. Below are some resources, examples I used when creating this file:

@sherwinski
Copy link
Contributor Author

sherwinski commented Jan 29, 2020

Hello @ikait 👋

I am trying to enable our users to import this project with SPM. I would love to get your opinion on how this Package.swift is organized and any issues/best practices you think I might have missed in the process. Thank you

@ikait
Copy link
Contributor

ikait commented Jan 30, 2020

Hello @sherwinski,

This is a great PR we've been waiting for. Indeed, we've already used ImgixSwift I forked and made it support SPM in our projects so I can provide some tips for this. These changes are almost good but some points will cause a import error as follows:

  • platforms: SPM doesn't have tvOS(.v9_2) as a supported version so you can specify tvOS("9.2") or choose a v9 or v10.
  • sources: SPM cannot import a target with source files contains mixed language so you can add a new target for NSString+ImgixSwiftMd5.h as like this page instructed. Or, rewrite it with Swift. In my forked project, I chose the rewrite way because the code is not so heavy.

@sherwinski
Copy link
Contributor Author

sherwinski commented Jan 31, 2020

Thank you @ikait for the insightful feedback and resources! I am trying to split up the library by swift and objc types, but am still working through some build errors at the moment. I may end up ultimately rewriting it all in swift, as you mentioned.

On a separate note - now that I can devote more time and attention to library, I would happily review/accept any pull requests for it. Improving imgix-swift is important to us and receiving direct feedback/contributions from its users is extremely valuable in achieving that. Also, you can expect my response time to be faster this time around 😁

@sherwinski
Copy link
Contributor Author

Hello @ikait, I've made a couple of changes in regards to your feedback and I think this is ready for release. Would love to get your thoughts on it especially since it sounds like you have gone through these steps on your own fork. Thank you

@ikait
Copy link
Contributor

ikait commented Feb 12, 2020

Hi @sherwinski, I confirmed your changes and I applied ImgIxSwift of this branch to my project. Now it works nicely. Perfect! 💯

@sherwinski
Copy link
Contributor Author

That's great! Thanks again for your help 😁

@sherwinski sherwinski merged commit 0150dc7 into master Feb 12, 2020
@sherwinski sherwinski deleted the spm-package-manifest branch February 12, 2020 21:08
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.

None yet

2 participants