Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concurrent networking #35

Merged
merged 7 commits into from
Aug 1, 2023
Merged

Concurrent networking #35

merged 7 commits into from
Aug 1, 2023

Conversation

Calibretto
Copy link
Contributor

@Calibretto Calibretto commented Jul 13, 2023

This PR splits the closure style request and the async request into two separate paths - this seems to be the most reliable way to handle it. A synchronous version has been removed as there's no reliable way to achieve it, due to threading issues (we can spend some more time investigating when it comes time to implement the dynamic context).

I'm not 100% comfortable with the way the resolver and the cache are interacting - I think it's maybe just the naming of things that's confusing me - if we have time before the beta release I'd like to go back and take a look at it.

@Calibretto Calibretto force-pushed the networking branch 2 times, most recently from e8b8121 to 0ee6bcf Compare July 20, 2023 13:06
@Calibretto Calibretto marked this pull request as ready for review July 20, 2023 13:07
@Calibretto Calibretto changed the title [WIP] concurrent networking Concurrent networking Jul 20, 2023
Copy link
Member

@fabriziodemaria fabriziodemaria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Before approving I'd like to fix the formatting warns if possible: I see that just running the swift-format script helps

@Calibretto Calibretto merged commit 83c5ccb into main Aug 1, 2023
2 checks passed
@Calibretto Calibretto deleted the networking branch August 1, 2023 14:21
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.

None yet

3 participants