search

pointfreeco / composable-core-location

A library that bridges the Composable Architecture and Core Location.
MIT License
107 stars 56 forks source link

`.failing` isn't enclosed in #if DEBUG thus breaks release builds #14

Closed andreyz closed 2 years ago

andreyz commented 2 years ago

Enclosing extension in #if DEBUG #endif will solve it. https://github.com/pointfreeco/composable-core-location/blob/95c32ab3677023aed0f5eb18c5c312bc31e296cb/Sources/ComposableCoreLocation/Failing.swift#L5

andreyz commented 2 years ago

Ehh! Just realised that we've misunderstood each other and the PR we've merged didn't help fix package building for Release.

The reason why this package fails to build for release is that Effect.failing is only defined for Debug in TCA, while our earlier discussion was around XCTFail (which is indeed stubbed for Release in XCTestDynamicOverlay).

I'm ready to make another PR. What's best plan of action:

Implement stubbed Effect.failing in ComposableCoreLocation or Enclose LocationManager.failing in #if DEBUG/#endif?

andreyz commented 2 years ago

Maybe even better solution is to remove #if DEBUG/#endif from TCA's definition of Effect.failing?

mbrandonw commented 2 years ago

Maybe even better solution is to remove #if DEBUG/#endif from TCA's definition of Effect.failing?

Yes, let's do that! These #if DEBUGs are a mess and I think this one is just left over from before we had XCTFail defined for release builds.

andreyz commented 2 years ago

OK! Then I'll make a PR on TCA repo. Once that gets merged, I can make a follow up PR here to pin this package to TCA revision of version containing the #if DEBUG/#endif removal from definition of Effect.failing.

mbrandonw commented 2 years ago

Sounds good!

Also if you need to be unblocked immediately I suppose you could wrap your failing effects in #if DEBUG in the meantime.

stephencelis commented 2 years ago

@andreyz We just released swift-composable-architecture 0.27.1, so feel free to submit that PR. We can also do it. Just wanted to give you an opportunity to first 😄

andreyz commented 2 years ago

Yeah! I'll only have time available in 5-6 hours, feel free to do it before.

andreyz commented 2 years ago

Updated the PR just now.