wwt / SwiftCurrent

A library for managing complex workflows in Swift
https://wwt.github.io/SwiftCurrent/
Apache License 2.0
308 stars 19 forks source link

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

Closed Richard-Gist closed 3 years ago

Richard-Gist commented 3 years ago

Linked Issue:

Checklist:

Richard-Gist commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

wiemerm commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.