wwt / SwiftCurrent

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

Replaces `WorkflowLauncher` and `thenProceed(with:)` with a new `WorkflowView` API, as discussed in #61 #179

Closed Tyler-Keith-Thompson closed 2 years ago

Tyler-Keith-Thompson commented 2 years ago

Linked Issue: Closes #156

Checklist:


If Applicable:


If Public API Has Changed:

codecov-commenter commented 2 years ago

Codecov Report

Merging #179 (94f06fe) into main (f69e8ac) will decrease coverage by 0.30%. The diff coverage is 87.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #179      +/-   ##
==========================================
- Coverage   91.28%   90.97%   -0.31%     
==========================================
  Files          92      102      +10     
  Lines        2375     2538     +163     
==========================================
+ Hits         2168     2309     +141     
- Misses        207      229      +22     
Impacted Files Coverage Δ
...xample/Views/Settings/SettingsViewController.swift 0.00% <0.00%> (ø)
...UI/Deprecations/WorkflowLauncherDeprecations.swift 0.00% <0.00%> (ø)
...rent_SwiftUI/ViewModifiers/NavigationWrapper.swift 100.00% <ø> (ø)
...rent_SwiftUI/Protocols/_WorkflowItemProtocol.swift 25.00% <25.00%> (ø)
...Example/Views/Profile/AccountInformationView.swift 79.71% <41.17%> (ø)
...ftUIExample/SwiftUIExample/SwiftUIExampleApp.swift 92.30% <80.00%> (ø)
...ws/Settings/SettingsOnboardingViewController.swift 88.23% <88.23%> (ø)
ExampleApps/SwiftUIExample/Views/ContentView.swift 93.75% <100.00%> (+1.44%) :arrow_up:
ExampleApps/SwiftUIExample/Views/LoginView.swift 94.33% <100.00%> (ø)
...rrent_SwiftUI/ResultBuilders/WorkflowBuilder.swift 100.00% <100.00%> (ø)
... and 8 more

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 5ad629b...94f06fe. Read the comment docs.

morganzellers commented 2 years ago

I'm liking this a lot. I think the main thing said against a style similar to this when originally discussing the SwiftUI API was that it didn't feel clear enough that this is an ordered sequence of FR's. I'm not really feeling that so much anymore with this. Having sat with the choice we made about the SwiftUI API, I don't think it was as crucial as we made it out to be.

Tyler-Keith-Thompson commented 2 years ago

We did change the public API but I don't know if there's a great way to have the compiler guide people to the new change. This is something the team should discuss before merging.

The changes to WorkflowItem and the lowering of the ACL on WorkflowLauncher are what lead me to think it won't be reasonable to guide people to WorkflowView.