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
Feature - Network Activity Indicator #1
Conversation
cc @jshier, @kcharwood, @kylef for review |
Once we get this through, I'll create all the documentation and get the repo opened up so I can start testing with Travis. |
Pod spec? |
I'm on it @jshier. 👍🏼 |
Podspec is up but won't build until Alamofire 3.2.0 is released. |
@@ -0,0 +1 @@ | |||
github "Alamofire/Alamofire" "master" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be repointed at ~> 3.2
once it is actually released.
Feature - Network Activity Indicator
This PR adds the
NetworkActivityIndicatorManager
class and all the tests to verify behavior. Code coverage is currently at 98% and looking good.The general approach is fairly similar to AFN with a simplification of not having to swizzle the
NSURLSessionTask.State
changes since we can rely on theRequest
methods directly. While this approach does not work if the user uses the task property directly toresume
orsuspend
the task, this should be able to be resolved with documentation. This is certainly a better tradeoff than swizzling the task state. AFN couldn't use this approach b/c there is no concept of aRequest
.@jshier and I have a huge conversation on the design choices of using
NSNotification
instead of closures on theManager
orRequest
classes. We decided that it is best in this case to use notifications since it is a broadcast that could have multiple observers.