Add support for HTTPS and more countries. #70
Conversation
src/Amazon.php
Outdated
| * @throws Exception\InvalidArgumentException | ||
| * @return Amazon | ||
| */ | ||
| public function __construct($appId, $countryCode = 'US', $secretKey = null, $version = null) | ||
| { | ||
| public function __construct($appId, $countryCode = 'US', $secretKey = null, $version = null, |
There was a problem hiding this comment.
when you make parameters multiline, all of them should be placed on separate lines
|
@Xerkus, sorry about that, didn't catch the Travis failure. All fixed now! |
src/Amazon.php
Outdated
| $countryCode = 'US', | ||
| $secretKey = null, | ||
| $version = null, | ||
| $useSsl = false |
There was a problem hiding this comment.
Do you think $useHttps would be better name?
There was a problem hiding this comment.
I went back and forth but don't really have a strong preference. Since you seem to prefer this option, I'll go ahead and change it.
There was a problem hiding this comment.
Okay, fixed. Apologies for missing an instance the first time I edited the file -- there should be a Travis build failure followed by a success.
There was a problem hiding this comment.
I see that Travis build 169 passed when it should have failed. This is puzzling; when I check out commit 619efaa and run tests, they fail as they should. Perhaps I made my second fix so quickly that the Travis build ran against the newer code both times.
There was a problem hiding this comment.
I don't have strong preference either. It is just that "SSL" irks me for unrelated reason - we don't really mean SSL transport, which is vulnerable and insecure, when we use that term.
Resolves #57.