pointfreeco / swift-clocks

⏰ A few clocks that make working with Swift concurrency more testable and more versatile.
MIT License
266 stars 17 forks source link

Remove AsyncAlgorithms #25

Closed brzzdev closed 10 months ago

brzzdev commented 10 months ago

throttle -> _throttle Remove select

mbrandonw commented 10 months ago

Hey @brzzdev, thanks for taking the time to do this.

Ideally we would update this dependency so that it supports 0.0.0..<2.0.0 so that we don't mess up people using older versions of AsyncAlgorithms. But also I don't think that's even possible because we can detect between 0.x and 1.x at compile time.

But also, I think we would prefer to just remove the AsyncAlgorithms dependency entirely, as well as the tests that use it. It's too much added complexity from a maintenance point of view for not much benefit.

Would you want to give that a shot?

brzzdev commented 10 months ago

Hi @mbrandonw, no worries. Happy to try and help :)

It looks like AsyncAlgorithms is only used in ClocksTests, so unless someone explicitly pulls in that test target, it shouldn't affect their version.

Tested this in a new project, pulling in swift-clocks only pulls down swift-concurrency-extras and xctest-dynamic-overlay.

Screenshot 2023-12-06 at 21 01 40

Still happy to try removing AsyncAlgorithms if you still think it's best, but I'd say there's value in testing how the clocks interact with it.

brzzdev commented 10 months ago

Screenshot 2023-12-06 at 21 05 32

To further test this, the same test app is happy to pull down 1.0.0 of AsyncAlgorithms even though the current production version of swift-clocks is relying on specific commit of it for it's testTarget.

stephencelis commented 10 months ago

@brzzdev I think we'd still prefer to remove the dependency entirely at this point. It was mainly helpful when first developing the library to validate against other code written by Apple.

brzzdev commented 10 months ago

Fair enough.

Besides deleting the AsyncAlgorithmsTests file, the only other test relying on it was testRunMultipleUnitsOfWork where I replaced AsyncTimerSequence with your clock.timer