Fix PHP 7 incompatibilities #2
Conversation
emirb
commented
Sep 22, 2017
emirb
commented
- Class Void renamed to VoidAdapter
- Updated Guzzle to 5.3.1 due to invalid syntax (don't use eventName as parameter and variable guzzle/guzzle#1393)
- Fixed outdated composer.lock
|
Can you please add Travis (or similar) continuous integration service to the repository? |
| * @param CurlHandler $ch optional initialized curl handle | ||
| * @param string $uri the URI to make the request to | ||
| * @param array $post the parameters to use for the POST body | ||
| * @param \GuzzleHttp\Ring\Client\CurlHandler $ch optional initialized curl handle |
There was a problem hiding this comment.
This is an actual Curl adapter that is not related to Guzzle. So this is an invalid change
There was a problem hiding this comment.
Can you please point me to the actual file where the adapter is defined?
There was a problem hiding this comment.
THIS file.. the one you changed and we are commenting on :)
There was a problem hiding this comment.
Sorry, my bad. I was confused by the fact that CurlHandler class does not exist. If it existed in this file then it'd be a violation of PSR-0 autoloading because of class name/file name mismatch.
By your comment, the parameter should be changed to \schibsted\payment\sdk\adapters\Curl. However, the thing is that the the parameter will actually be a resource (check curl_init return), so the type-hint against anything except resource would be misleading. As this is just a part of the care-requiring codebase, I'll revert my changes, to not open a Pandora box. :)
There was a problem hiding this comment.
The value that is returned does not have a class, it is type hinted only as "resource" in the php docs : http://php.net/manual/en/function.curl-init.php so I am not sure what typehint would would here. That docblock line was written like that for human to read not IDE I guess.
| @@ -16,6 +16,7 @@ | |||
| protected $_sdk = null; | |||
|
|
|||
| protected $_sdk_class = ''; | |||
| protected $name; | |||
There was a problem hiding this comment.
$name field has to be, and is, set on the sub classes. Abstract field, if there was such a thing.
There was a problem hiding this comment.
This was mostly for IDE complaints :-)
|
Ok Ill merge this and look at the issues with travis and the typehinting. |
|
Thanks @alkemann! |
