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

Uppercase the method to ensure compatibility #207

Merged
merged 2 commits into from
Apr 19, 2016
Merged

Conversation

rmccue
Copy link
Collaborator

@rmccue rmccue commented Apr 19, 2016

Easy fix to improve usability. :) Fixes #204.

@rmccue rmccue added this to the 1.7 milestone Apr 19, 2016
@@ -572,6 +572,9 @@ protected static function set_defaults(&$url, &$headers, &$data, &$type, &$optio
$options['data_format'] = 'body';
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

$type var should be changed to uppercase at the start of this function since $type var is used to define data format.
Let me know if you want me to create PR or update it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch!

@ccrims0n
Copy link
Contributor

@rmccue I think we should also change $type in request_multiple() since $request['options']['type'] is set before the call of set_defaults().

@rmccue
Copy link
Collaborator Author

rmccue commented Apr 19, 2016

@73SL4 It operates by reference, so that'll get changed anyway; the logic in request_multiple() is just for merging the "global" options into the per-request options.

@rmccue rmccue merged commit a71ab70 into master Apr 19, 2016
@rmccue rmccue deleted the uppercase-method branch April 19, 2016 06:11
@ccrims0n
Copy link
Contributor

@rmccue set_defaults() is only uppercasing $type var.
request_multiple() is setting $request['options']['type'] which is not changed to uppercase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small caps http verb don't work as expected
2 participants