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

Add session delegate to initializer #44

Merged
merged 6 commits into from
Jan 19, 2024
Merged

Conversation

vani2
Copy link
Collaborator

@vani2 vani2 commented Nov 17, 2023

  • Set min deployment target to 13.0
  • Removed block-based API from Example project
  • Renamed func data(for request:, delegate:) async throws -> (Data, URLResponse) to func data(forRequest:, delegate:) async throws -> (Data, URLResponse) because it conflicts with Foundation.
  • Added optional parameter delegate: URLSessionDelegate to URLSessionClient init.

@vani2 vani2 self-assigned this Nov 17, 2023
@vani2 vani2 marked this pull request as draft November 17, 2023 21:52
@vani2 vani2 marked this pull request as ready for review November 28, 2023 21:00
@vani2 vani2 force-pushed the feature/init-with-delegate branch 2 times, most recently from 19d2d73 to e596d49 Compare November 28, 2023 21:14
@@ -20,7 +20,7 @@ extension URLSession {
/// - delegate: Delegate to get events about request (NOT WORKING💀)
/// - Returns: Tuple with Data and URLResponse
public func data(
for request: URLRequest,
forRequest request: URLRequest,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a change in the public API, so I think you should update the major version in the podspec file. Or at least a minor version 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this extension at all now. it was implemented at time when URLSession had their async/await methods ready for use only since iOS 15. so to make URLSessionClient implementataion for iOS 13 and easy to read i have made this extension with async/await methods.

now this methods are in URLSession available since iOS 13 so you can just delete this file and it will work justs fine

Copy link
Contributor

@laqiguy laqiguy Dec 20, 2023

Choose a reason for hiding this comment

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

and also now you can get rid of AsyncAwaitHelper file too
#42 will be fixed also

@@ -20,7 +20,7 @@ extension URLSession {
/// - delegate: Delegate to get events about request (NOT WORKING💀)
/// - Returns: Tuple with Data and URLResponse
public func data(
for request: URLRequest,
forRequest request: URLRequest,
Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need this extension at all now. it was implemented at time when URLSession had their async/await methods ready for use only since iOS 15. so to make URLSessionClient implementataion for iOS 13 and easy to read i have made this extension with async/await methods.

now this methods are in URLSession available since iOS 13 so you can just delete this file and it will work justs fine

@@ -20,7 +20,7 @@ extension URLSession {
/// - delegate: Delegate to get events about request (NOT WORKING💀)
/// - Returns: Tuple with Data and URLResponse
public func data(
for request: URLRequest,
forRequest request: URLRequest,
Copy link
Contributor

@laqiguy laqiguy Dec 20, 2023

Choose a reason for hiding this comment

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

and also now you can get rid of AsyncAwaitHelper file too
#42 will be fixed also

- remove URLSession extension
- remove AsyncAwaitHelper
- remove ProgressWrapper
- remove AsyncAwaitHelper tests
@laqiguy
Copy link
Contributor

laqiguy commented Dec 26, 2023

@vani2 somehow tests always fails. i can't figure out why so

@laqiguy laqiguy merged commit c7d75f8 into master Jan 19, 2024
3 checks passed
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