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.

Sign up for GitHub

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

Jump to bottom

Add arrayParameters to RequestType protocol #125

Merged
merged 5 commits into from Mar 4, 2016
Merged

Add arrayParameters to RequestType protocol #125

merged 5 commits into from Mar 4, 2016

Conversation

toshi0383
Copy link
Contributor

Addresses #124

@ishkawa
Copy link
Owner

ishkawa commented Feb 15, 2016

Thank you for the PR.

I'm considering to change the type of parameters to AnyObject in the future version, but it is breaking change. To keep compatibility in current major version, adding secondary property seems to be a good solution.

IMO, var objectParameters: AnyObject is better than var arrayParameters: [JSON] because NSJSONSerialization receives AnyObject and [JSON] cannot express nested arrays.

@ishkawa
Copy link
Owner

ishkawa commented Feb 15, 2016

Could you merge master to fix CI?

@toshi0383
Copy link
Contributor Author

Rebased to master.
Also changed arrayParameters to objectParameters because you're so right ! 😁

@toshi0383 toshi0383 closed this Feb 15, 2016
@toshi0383 toshi0383 reopened this Feb 15, 2016
@toshi0383
Copy link
Contributor Author

Looks like OHHTTPStubs has carthage build issue.
This little tweak solved the Build failure on my machine(after carthage build and snapshot reset_simulators), but I'm not sure it's the right solution.

diff --git a/OHHTTPStubs/OHHTTPStubs.xcodeproj/xcshareddata/xcschemes/OHHTTPStubs tvOS Framework.xcscheme b/OHHTTPStubs/OHHTTPStubs.xcodeproj/xcshareddata/xcschemes/OHHTTPStubs tvOS Framework.xcscheme
index cde7ba1..e75d1e0 100644
--- a/OHHTTPStubs/OHHTTPStubs.xcodeproj/xcshareddata/xcschemes/OHHTTPStubs tvOS Framework.xcscheme      
+++ b/OHHTTPStubs/OHHTTPStubs.xcodeproj/xcshareddata/xcschemes/OHHTTPStubs tvOS Framework.xcscheme      
@@ -3,8 +3,8 @@
    LastUpgradeVersion = "0710"
    version = "1.3">
    <BuildAction
-      parallelizeBuildables = "YES"
-      buildImplicitDependencies = "YES">
+      parallelizeBuildables = "NO"
+      buildImplicitDependencies = "NO">

ishkawa
ishkawa reviewed Mar 4, 2016
View reviewed changes
APIKit/RequestType.swift
if parameters.count > 0 {
URLRequest.HTTPBody = try requestBodyBuilder.buildBodyFromObject(parameters)
} else if let count = objectParameters.count where count > 0 {
URLRequest.HTTPBody = try NSJSONSerialization.dataWithJSONObject(objectParameters, options: NSJSONWritingOptions.PrettyPrinted)
Copy link
Owner

Choose a reason for hiding this comment

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

This should be URLRequest.HTTPBody = try requestBodyBuilder.buildBodyFromObject(objectParameters).

@toshi0383
Copy link
Contributor Author

Thanks for pointing out.
Fixed missing URL query support in 07d4d76 .

@ishkawa
Copy link
Owner

ishkawa commented Mar 4, 2016

Nice ✨
I'll merge this PR into master after fixing CI issue on a merge branch.

@ishkawa ishkawa merged commit 07d4d76 into ishkawa:master Mar 4, 2016
@toshi0383 toshi0383 deleted the ts-array-parameters branch March 4, 2016 08:41
@ishkawa ishkawa mentioned this pull request Mar 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@toshi0383 @ishkawa