memoize user's credentials and refactoring #7
Conversation
* assert() ==> NSParameterAssert() * using _cmd instead of selector literals where appropriate * JAHPQNSURLSessionDemuxTaskInfo extracted to a separate file * a few typedefs introduced for convenience * a few local variables extracted
* Extracted some invariants as local variables * Removed some duplicated calls
Pods directory is not included to .gitignore So I’m not ignoring these changes and committing them.
…iption to the readme. Added some pitfalls I’ve faced to the readme too.
| self.authAlertView.alertViewStyle = UIAlertViewStyleLoginAndPasswordInput; | ||
| [self.authAlertView show]; | ||
|
|
||
| // JAHPDidCancelAuthenticationChallengeHandler logCancelEvent = |
There was a problem hiding this comment.
Why did you comment this out?
There was a problem hiding this comment.
That's what I've introduced before noticing the #define USE_PROTOCOL_FOR_CANCELLATION 0 . Forgot to delete. Will fix that.
| -(void)handleAuthChallenge:(NSURLAuthenticationChallenge*)challenge | ||
| forProtocol:(JAHPAuthenticatingHTTPProtocol*)authenticatingHTTPProtocol | ||
| { | ||
| // TODO: maybe fix leaking `self` |
There was a problem hiding this comment.
Where is the leaking self?
There was a problem hiding this comment.
Same thing. Old irrelevant comment I've written before the refactoring happened. Will delete that.
| self.authAlertView.alertViewStyle = UIAlertViewStyleLoginAndPasswordInput; | ||
| [self.authAlertView show]; | ||
|
|
||
| return nil; |
There was a problem hiding this comment.
I'm fine with adding a logging challenge handler. I think that would make exploration a little simpler.
There was a problem hiding this comment.
I believe, if this function returns non-nil, the authenticatingHTTPProtocol:didCancelAuthenticationChallenge: method will end up not being called. Which is unwanted for the #if USE_PROTOCOL_FOR_CANCELLATION demo branch.
|
I'll have to do a more in-depth review later tonight. |
|
There's still a ton of noise in this PR from reformatting that isn't strictly necessary. If you want different formatting for this project, I'm fine with it, but let's make that a separate PR. |
Ok. I understand. I'll try to split the changes later. |
|
Reverted original formatting in #9 Tested with
|
|
Closing this since a more recent PR is available #9 |
pod installcommand. Not changing the.gitignorepolicy, so I've committed them too.Basically, the reviewer may ignore the contents of the
Podsdirectory.