Skip to content

[PHP] add Accept encoding using CURLOPT_ENCODING#5525

Merged
wing328 merged 1 commit intoswagger-api:masterfrom
mguan-trulia:add-php-client-allow-encoding
May 6, 2017
Merged

[PHP] add Accept encoding using CURLOPT_ENCODING#5525
wing328 merged 1 commit intoswagger-api:masterfrom
mguan-trulia:add-php-client-allow-encoding

Conversation

@mguan-trulia
Copy link
Copy Markdown
Contributor

@mguan-trulia mguan-trulia commented May 2, 2017

PR checklist

  • Read the contribution guidelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

This change is to implement PHP client for allowing encoding headers.
#3365

@wing328 wing328 added this to the v2.2.3 milestone May 2, 2017
@wing328
Copy link
Copy Markdown
Contributor

wing328 commented May 2, 2017

@mguan-trulia thanks for the PR.

cc @arnested @baartosz @dkarlovi

* Allow Curl encoding header
*
* @var bool
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$allowEncoding?

Copy link
Copy Markdown
Contributor

@dkarlovi dkarlovi left a comment

Choose a reason for hiding this comment

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

Only one style change, overall the change is OK.

* Allow Curl encoding header
*
* @var bool
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

$allowEncoding?

@dkarlovi
Copy link
Copy Markdown
Contributor

dkarlovi commented May 2, 2017

IMO, this PR is pointing out the problem with building our own HTTP client, these kinds of features will trickle down one by one as someone needs them when in fact we should promote using #5190 and/or adding #5305.

@mguan-trulia mguan-trulia force-pushed the add-php-client-allow-encoding branch from a6a9658 to 9c1a4b4 Compare May 2, 2017 20:19
@mguan-trulia
Copy link
Copy Markdown
Contributor Author

Thanks for the comments @dkarlovi , I've made the variable name change as suggested. Thank you for letting me know about the Guzzle option. That is preferable for what I'm working on going forward.

@wing328 wing328 merged commit 918343b into swagger-api:master May 6, 2017
davidgri pushed a commit to davidgri/swagger-codegen that referenced this pull request May 11, 2017
@wing328 wing328 changed the title issue #3365 adding accept encoding for PHP [PHP] add Accept encoding using CURLOPT_ENCODING Jun 5, 2017
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.

3 participants