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

Expands platform support to include macOS, watchOS, and tvOS. #94

Merged
merged 14 commits into from
Aug 5, 2021

Conversation

Richard-Gist
Copy link
Collaborator

@Richard-Gist Richard-Gist commented Aug 3, 2021

Linked Issue:

Checklist:

@Richard-Gist Richard-Gist added the enhancement New feature or request label Aug 3, 2021
@Richard-Gist Richard-Gist added this to the SwiftUI Support milestone Aug 3, 2021
@Richard-Gist Richard-Gist requested a review from a team as a code owner August 3, 2021 20:50
@Richard-Gist
Copy link
Collaborator Author

EOD: Looks like the repo we are using to validate has something weird set up wrong in the Watch app for Cocoapods. When I converted @wiemerm 's project to a Cocoapod project, watch ran for me.

We may consider dropping watchOS support in CocoaPods for now so we can push this out, but also we could fix it for 12.4. We could also just set the watch version high enough that you need 12.5 to use it?

morganzellers
morganzellers previously approved these changes Aug 4, 2021
@morganzellers
Copy link
Contributor

morganzellers commented Aug 4, 2021

EOD: Looks like the repo we are using to validate has something weird set up wrong in the Watch app for Cocoapods. When I converted @wiemerm 's project to a Cocoapod project, watch ran for me.

We may consider dropping watchOS support in CocoaPods for now so we can push this out, but also we could fix it for 12.4. We could also just set the watch version high enough that you need 12.5 to use it?

I would be up for dropping watch support from CocoaPods temporarily or bumping the version high enough to get past 12.4. I know @Tyler-Keith-Thompson had a few ideas yesterday of how to get around the XCTest build error. Maybe we could circle back with one of those approaches in the future

@morganzellers
Copy link
Contributor

morganzellers commented Aug 4, 2021

Pushed a change that upped the watchOS version in the .podspec to 7.4 (requires Xcode 12.5) to see what the CI does. Running the pod lib lint locally gave me an error about missing a watchOS 7.4 simulator (makes sense, I don't have Xcode 12.5) so I'm curious if this appeases the pipeline or if we'll need to remove CocoaPods watchOS support for now

EDIT: Pipeline had the same issue, so I'm going to push a commit that removes support for watchOS from CocoaPods - we can discuss if we want to keep it or not in standup

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2021

Codecov Report

Merging #94 (127247e) into main (562cd51) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #94   +/-   ##
=======================================
  Coverage   94.70%   94.71%           
=======================================
  Files          62       62           
  Lines        1606     1608    +2     
=======================================
+ Hits         1521     1523    +2     
  Misses         85       85           
Impacted Files Coverage Δ
Sources/SwiftCurrent/TestOnly/TestOnly.swift 90.00% <ø> (ø)
...urrent_UIKit/Extensions/LaunchStyleAdditions.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d555fab...127247e. Read the comment docs.

@Richard-Gist Richard-Gist added the documentation Improvements or additions to documentation label Aug 4, 2021
@wiemerm
Copy link
Contributor

wiemerm commented Aug 4, 2021

We may consider dropping watchOS support in CocoaPods for now so we can push this out, but also we could fix it for 12.4. We could also just set the watch version high enough that you need 12.5 to use it?

Regardless of whether CocoaPods remove(d/s) support for watchOS, we should consider bumping the Package.swift file watchOS support to be high enough to need 12.5 (watchOS 7.4) if possible. Either we gain that you can't use SwiftCurrent to build watchOS apps prior to that version (and therefore on Xcode pre-12.5) or we at least have an indicator that we can't guarantee it's usable prior to that version.

Update: It doesn't appear possible. We can only select whole number versions.

Richard-Gist and others added 3 commits August 4, 2021 09:49
…or xcode 12.4 - MW RAG

Co-authored-by: Megan Wiemer <megan.wiemer@wwt.com>
…elsewhere - MW RAG

Co-authored-by: Megan Wiemer <megan.wiemer@wwt.com>
morganzellers
morganzellers previously approved these changes Aug 4, 2021
@Richard-Gist
Copy link
Collaborator Author

Things still missing in this, updating the supported versions doc (SECURITY.md). And we need the SPM validation step to actually validate these things as well.

@Richard-Gist
Copy link
Collaborator Author

EOD: I can't figure out how to use swift build to build against macOS and the same for trying to build against certain destinations. I would really want this to work:

# Confirm OrchestrationResponders can build for their destinations
    sh('xcodebuild -scheme SwiftCurrent_UIKit')
    sh('xcodebuild -scheme SwiftCurrent_UIKit -sdk iphonesimulator -destination"platform=iOS Simulator,name=iPhone 12"')
    sh('xcodebuild -scheme SwiftCurrent_UIKit -sdk appletvsimulator -destination"platform=tvOS Simulator,name=Apple TV"')
    sh('xcodebuild -scheme SwiftCurrent_UIKit -sdk macosx -ARCHS="x86_64h" -destination"platform=macOS,variant=Mac Catalyst"')
    sh('xcodebuild -scheme SwiftCurrent_UIKit -sdk iphonesimulator -destination"platform=macOS,variant=Mac Catalyst"')

I have done a lot to try and curate things a little more into better testing.

@brianlombardo brianlombardo merged commit aa166ed into main Aug 5, 2021
@brianlombardo brianlombardo deleted the testing_platforms branch August 5, 2021 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants