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

Swift Package Manager support #32

Merged
merged 9 commits into from
May 2, 2020
Merged

Swift Package Manager support #32

merged 9 commits into from
May 2, 2020

Conversation

martinpucik
Copy link
Contributor

Changes

  • Added Package.swift to support Swift Package Manager without changes to project structure, so hopefully nothing will break for Cocoapods and Carthage
  • Changed imports in public header file

Additional info

build, tests, analyze and pod lib lint were all green on my local machine, lets wait for CI and see if everything works

Copy link

@Kaspik Kaspik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Kaspik
Copy link

Kaspik commented Apr 30, 2020

@garrettmoon / @jparise Hey! I know it might not be top priority, but we would love to use whole PINRemoteImage in module (framework) through SPM so we are ready to prepare it for SPM for you.

Martin started here, will continue with PINCache etc., but it would be nice if you could review this and stay up-to-date with our PRs and keep merging them if you think this is a good idea.

Thanks!

@jparise
Copy link
Collaborator

jparise commented Apr 30, 2020

What sort of CI-level check should we have in place to verify that the packaging is (and remains) correct?

Example project to test Pods integration
Build Example project on CI
Added some code from PINOperation to compile in example project
@martinpucik
Copy link
Contributor Author

martinpucik commented May 2, 2020

Hey @jparise thank you for taking a look at this!

I added a test target to SPM to run current tests in the package too, that should help verify it's behaving properly.

And I added an Example project and build step on CI where we can verify the Cocoapods integration is still working properly after adding SPM and making changes to PINOperation source.

Let me please know if you think any other checks or changes are necessary

Copy link
Collaborator

@jparise jparise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Tagging @garrettmoon for a final review.

@jparise jparise requested a review from garrettmoon May 2, 2020 21:12
Copy link

@Kaspik Kaspik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job Martin! 🚀

Copy link
Collaborator

@garrettmoon garrettmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! Thank you so much for adding this!

@garrettmoon garrettmoon merged commit 88f6e3a into pinterest:master May 2, 2020
@Kaspik
Copy link

Kaspik commented May 2, 2020

@garrettmoon Thanks for merge! Would you mind bumping version and releasing new one + tag to make this available for PINCache and other parts as SPM already?

@jparise
Copy link
Collaborator

jparise commented May 2, 2020

@Kaspik I think we'd be in a good place to release a new version once #31 is also addressed. That would give us good overall platform and packaging support.

@martinpucik martinpucik deleted the spm branch May 3, 2020 10:05
@Kaspik
Copy link

Kaspik commented Jun 30, 2020

@jparise #31 merged as well, do you think we could release and tag new version for this? We would like to continue with SPM for PINCache and PINRemoteImage. :)

@garrettmoon
Copy link
Collaborator

Thank you both so much for the work! I'll get out a new release.

@garrettmoon
Copy link
Collaborator

Published!

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

4 participants