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

[NATIVESDK-156] Allow configuration of async request runModes #9

Merged
merged 6 commits into from Jun 14, 2014
Merged

[NATIVESDK-156] Allow configuration of async request runModes #9

merged 6 commits into from Jun 14, 2014

Conversation

vishalduggal
Copy link
Contributor

Changes in APSHTTPClient for NATIVESDK-156

@matt-langston
Copy link
Contributor

@vishalduggal has this been tested with aps_sdk? I think this teasing is blocked by NATIVESDK-150 and NATIVESDK-155. I am the owner of these issues, so the ball is in my court and I will resolve it so that you can test accordingly.

This seems like an excellent improvement, and a clear step in the right direction. Can you provide a few assertions to the code that communications your assumptions (specifically pre and post conditions) and maybe a few XCTest unit tests in the provided unit test target that communicates your theory of operation?

This would help me understand your thinking and design rationale. Don't get me wrong, I definitely like the direction, but I want/need to make sure that my understanding of what you are trying to accomplish matches with your expectations.

matt-langston added a commit that referenced this pull request Jun 14, 2014
Good job on this @vishalduggal. The unit tests you provided made this easy to test - thank you.
@matt-langston matt-langston merged commit 76550a2 into tidev:master Jun 14, 2014
@matt-langston
Copy link
Contributor

Great job on this @vishalduggal. The unit tests you provided made your PR easy to test.

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

2 participants