Skip to content

Conversation

@kevin-kp
Copy link
Contributor

This PR is an effort to fix the strict concurrency warnings when adding the swift setting StrictConcurrency (not included in the commit).
You can test this by adding the following to the necessary targets:

swiftSettings: [
    .enableExperimentalFeature("StrictConcurrency")
]

The only two warnings that are left are HTTPBodyEncoder and HTTPBodyDecoder.
This is because Sendable conformance is iOS >= 16.0 for JSONEncoder and JSONDecoder and I'm not sure what the best approach is to resolve these two warnings.

@kevin-kp kevin-kp marked this pull request as draft June 12, 2024 11:03
@joshuawright11
Copy link
Owner

joshuawright11 commented Jun 12, 2024

Thanks @kevin-kp! Great work 💯 couple Qs for you - (caveat - I haven't dived into Sendable a ton yet so they may just be misunderstandings on my part.)

Regarding JSONEncoder - would a @preconcurrency import Foundation work for versions less than iOS 16?

@joshuawright11
Copy link
Owner

LGTM - feel free to change from Draft to Open and I'll approve

# Conflicts:
#	Papyrus/Sources/PapyrusRequest.swift
#	Papyrus/Sources/PapyrusResponse.swift
@kevin-kp
Copy link
Contributor Author

@joshuawright11 it actually isn't ready yet as the Provider is still not Sendable (which i'm currently getting warnings from).

@kevin-kp kevin-kp marked this pull request as ready for review July 11, 2024 17:05
@kevin-kp
Copy link
Contributor Author

kevin-kp commented Jul 11, 2024

@joshuawright11 sorry for the delay, I had doubts of making provider an actor so I created a mutex instead.
I haven't done extensive tests as I got the issue mentioned in #59 which popped up when updating from 0.6.10 to 0.6.16

@kevin-kp kevin-kp changed the title Draft: Fix strict concurrency warnings Fix strict concurrency warnings Jul 11, 2024
@lutes1
Copy link
Contributor

lutes1 commented May 14, 2025

Any updates to this PR? @kevin-kp @joshuawright11 do you guys need any help?

@kevin-kp
Copy link
Contributor Author

@lutes1
As I didn't receive any response anymore, I just fixed the issues on the main branch of my fork and use that one instead

@lutes1
Copy link
Contributor

lutes1 commented May 15, 2025

Ah, do you mind creating another PR from your main branch? Maybe we can get @joshuawright11 to merge it and continue to maintain this repo.

I think it's important for this repo to succeed considering the state of api clients in Swift

@kevin-kp kevin-kp mentioned this pull request May 19, 2025
@kevin-kp
Copy link
Contributor Author

Closing this PR in favor of #72

@kevin-kp kevin-kp closed this May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants