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

SPM support #47

Merged
merged 6 commits into from Feb 17, 2021
Merged

SPM support #47

merged 6 commits into from Feb 17, 2021

Conversation

mateuszmackowiak
Copy link
Contributor

@mateuszmackowiak mateuszmackowiak commented Jun 7, 2020

SPM support for the library.
I have changed the sample json file to use multilane string to be compatible with sim prior swift 5.3 (resources).
Both Carthage and pods version are still supported.
SPM package contains passing tests and I have defined other supported platforms iOS(.v8), .macOS(.v10_15), .watchOS(.v4), .tvOS(.v11)

Fixes #11.

@1ec5
Copy link
Collaborator

1ec5 commented Sep 16, 2020

This would fix #11.

Copy link
Owner

@ceeK ceeK left a comment

Choose a reason for hiding this comment

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

Hi @mateuszmackowiak. Firstly, thank you for taking the time to work on this pull request, and apologies that it has taken me so long to follow up.

I'd like to get this merged ASAP as it appears to be in high demand. One thing that stood out to me compared to the other SwiftPM PRs is all of the .swiftpm files. I tried to figure out whether these are necessary to checkin and it appears people usually add the .swiftpm directory to their .gitignore. I found the SPM folks doing it themselves here.

Unless you have a reason for checking this in, would you be able to add .swiftpm to the .gitignore and remove the files under the .swiftpm directory?

Package.swift Outdated Show resolved Hide resolved
@@ -4,8 +4,8 @@

# Solar

[![Version](https://img.shields.io/cocoapods/v/Solar.svg?style=flat)](http://cocoapods.org/pods/Solar) [![Carthage compatible](https://img.shields.io/badge/Carthage-compatible-4BC51D.svg?style=flat)](https://github.com/Carthage/Carthage) [![Build Status](https://travis-ci.org/ceeK/Solar.svg?branch=master)](https://travis-ci.org/ceeK/Solar)
[![MIT licensed](https://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/hyperium/hyper/master/LICENSE)
[![Version](https://img.shields.io/cocoapods/v/Solar.svg?style=flat)](http://cocoapods.org/pods/Solar) [![Carthage compatible](https://img.shields.io/badge/Carthage-compatible-4BC51D.svg?style=flat)](https://github.com/Carthage/Carthage) [![POD compatible](https://img.shields.io/badge/Cocoapods-compatible-4BC51D.svg?style=flat)](https://cocoapods.org) [![SPM compatible](https://img.shields.io/badge/SPM-compatible-4BC51D.svg?style=flat)](https://swift.org/package-manager/) [![Build Status](https://travis-ci.org/ceeK/Solar.svg?branch=master)](https://travis-ci.org/ceeK/Solar)
Copy link
Owner

Choose a reason for hiding this comment

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

Awesome! Thank you 🙌

}()

override func setUpWithError() throws {
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Package.swift Outdated Show resolved Hide resolved
@1ec5 1ec5 requested a review from ceeK February 17, 2021 00:35
@1ec5
Copy link
Collaborator

1ec5 commented Feb 17, 2021

Unless you have a reason for checking this in, would you be able to add .swiftpm to the .gitignore and remove the files under the .swiftpm directory?

Done. I also made the minimum deployment targets more consistent among CocoaPods, Carthage, and SPM, though #29 tracks supporting more of the platforms with Carthage.

mateuszmackowiak and others added 5 commits February 16, 2021 18:31
Solar requires macOS 10.10, tvOS 9, or watchOS 3 when using SPM and macOS 10.9 or watchOS 3 when using Carthage for consistency with CocoaPods.
@ceeK
Copy link
Owner

ceeK commented Feb 17, 2021

LGTM!

@ceeK ceeK merged commit f0a693e into ceeK:master Feb 17, 2021
@ceeK
Copy link
Owner

ceeK commented Feb 17, 2021

And thank you @1ec5 for going through all this! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Swift Package Manager support
3 participants