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 support #25

Merged
merged 6 commits into from
Jan 15, 2020
Merged

Add tvOS support #25

merged 6 commits into from
Jan 15, 2020

Conversation

mdyson
Copy link
Contributor

@mdyson mdyson commented Nov 26, 2019

  • Adds tvOS support to the Podspec, swift package and as a scheme for carthage support
  • allows all tests to run as expected using the tvOS sdk

@giginet
Copy link
Owner

giginet commented Jan 14, 2020

@mdyson Sorry, I missed this PR such a long time 🙏

Copy link
Owner

@giginet giginet left a comment

Choose a reason for hiding this comment

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

@mdyson Looks great. Thanks!

I'll release newer version after some fixes.
Sorry for watiting you.

Package.swift Outdated
@@ -18,5 +18,11 @@ let package = Package(
.testTarget(
name: "CrossroadTests",
dependencies: ["Crossroad"]),
.target(
name: "Crossroad-tvOS",
Copy link
Owner

Choose a reason for hiding this comment

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

I think you need not add targets for each platforms on Package.swift.

The required changes are only L8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, I have dropped the other changes in this file.

import UIKit
import Crossroad
import XCTest
@testable import Crossroad
Copy link
Owner

Choose a reason for hiding this comment

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

Could you drop @testable?

I want to test whether these classes arepublic .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, I've changed it to remove @testable. There were a few spots where it is required though since the code is using code marked with internal so I had to leave those test files with it included.

@@ -8,6 +8,7 @@ before_script:
- swiftlint --strict
matrix:
include:
- env: NAME=Xcode DESTINATION="platform=iOS Simulator,name=iPhone 8"
- env: NAME=Xcode DESTINATION="platform=iOS Simulator,name=iPhone 8" SCHEME="Crossroad"
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer to rename to Crossroad-iOS. I'll rename this after merging this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, great! I will leave this one alone for you to change.

@giginet
Copy link
Owner

giginet commented Jan 15, 2020

Thanks!
I'll release newer version soon.

@giginet giginet merged commit 7427b56 into giginet:master Jan 15, 2020
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.

2 participants