Skip to content

Add URLSessionTaskMetrics #2886

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

Merged
merged 2 commits into from
Oct 1, 2020
Merged

Conversation

jshier
Copy link
Contributor

@jshier jshier commented Sep 28, 2020

Starts fixing SR-11518 and makes a port of Alamofire much easier.

This PR adds URLSessionTaskMetrics and its associated types (URLSessionTaskTransactionMetrics, etc.), as well as the urlSession(_:task:didFinishCollecting:) URLSessionTaskDelegate method. It does not include:

  • Documentation
    • (If there's a standard way of copying documentation, I'm happy to do that.)
  • Tests
  • Implementation

@MaxDesiatov

This comment has been minimized.

@MaxDesiatov
Copy link
Contributor

@swift-ci please test Linux platform

@@ -128,6 +128,8 @@ public protocol URLSessionTaskDelegate : URLSessionDelegate {
func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?)

func urlSession(_ session: URLSession, task: URLSessionTask, willBeginDelayedRequest request: URLRequest, completionHandler: @escaping (URLSession.DelayedRequestDisposition, URLRequest?) -> Void)

func urlSession(_ session: URLSession, task: URLSessionTask, didFinishCollecting metrics: URLSessionTaskMetrics)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting seems to be inconsistent here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other formatting is inconsistent: the first two methods are indented 4 spaces, the others 5. 4 seems correct.

Copy link
Contributor

@MaxDesiatov MaxDesiatov Sep 28, 2020

Choose a reason for hiding this comment

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

Sure, the nit is about the rest of the code being inconsistent, not yours 😁

Good opportunity to make the rest of the functions in this protocol use 4-spaces indentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Fixed all of the spacing issues in this file in 75bcd79.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@MaxDesiatov

This comment has been minimized.

1 similar comment
@MaxDesiatov
Copy link
Contributor

@swift-ci please test

@millenomi millenomi merged commit 7cc937e into swiftlang:main Oct 1, 2020
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