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

Add tvOS and watchOS deployment targets #49

Merged
merged 4 commits into from Jun 16, 2018
Merged

Add tvOS and watchOS deployment targets #49

merged 4 commits into from Jun 16, 2018

Conversation

thebarndog
Copy link
Contributor

@thebarndog thebarndog commented Jun 13, 2018

Obligatory Office GIF

dxocwugdzryg4

What

Adds tvOS and watchOS targets and made the schemes shared so Carthage can access them. Also added deployment targets to the podspec

The Why

TL;DR I was working on this: feathersjs-ecosystem/feathers-swift#29 and saw this awesome library but it didn't have tvOS/watchOS support for Carthage so I added it! But also because I said so!

Pending

One other change I'd like to see/might put a PR up for is adding testing targets for each deployment target. They'd all point to the same testing bundle and they'd let each scheme run the tests themselves which ensures tests pass on any given OS. And also Linux support would be cool, down to put a PR up for that as well.

@khanlou
Copy link
Owner

khanlou commented Jun 13, 2018

This is cool, thanks for the pull request. How does this change the process for various types of changes we might make in the future? For example, if there's a new file, what do I need to do to make sure this doesn't break?

@thebarndog
Copy link
Contributor Author

thebarndog commented Jun 14, 2018

I think all you'd have to do is make sure that the file is added to each target. If you add test targets for each framework target so that its a 1:1 ratio (where each test target points to the same test bundle), then if you forget to add a file to a particular build target, those tests should fail. I can do that in another PR if you'd like.

The way I do it is a bit roundabout and you do end up with 8 targets to manage vs two but I find it's a bit more explicit and easier to keep track of when each target is designated for a particular OS.

@thebarndog
Copy link
Contributor Author

I'm working on a framework scaffolding tool too, won't be done for a while, but that should ease the pain of manually setting up the targets individually which is probably the biggest pain, along with the manual steps of integrating Carthage

@khanlou
Copy link
Owner

khanlou commented Jun 16, 2018

Okay, I'm going to go ahead and merge this. Might ping you from time to time if there's an issue with it. Open to PRs for linux and other platform tests too.

@khanlou khanlou merged commit 9100518 into khanlou:master Jun 16, 2018
@thebarndog
Copy link
Contributor Author

Definitely! My email is on my profile so I don't respond to notifications (which happens a lot) you can use that if something urgent/extraneous comes up. I'm using this library too for the next version of Feathers Swift so I might uncover a thing or two.

@khanlou khanlou mentioned this pull request Feb 3, 2019
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