Let's see if we can pull the latest changes #6
Conversation
…rs for child classes. Added RequestInterface to improve testability/strong typing. PSR.
…e testability and SRP
…roke apart Card and OutputSpeech.
to maintain consistency with other setters.
…his a while ago...
…his a while ago...
…ted; just returning instead of throwing if [intent][slots] is missing
|
For one, I am unsure if Purifier is needed. All the data from Amazon is very strict - it's either a predefined value or a predefined format we know about. We should be able to avoid a library and purify by checking for explicit data types... Certificate will always be a URL |
There was a problem hiding this comment.
These changes give the library a much cleaner/more polished feel, and add some valuable features, like card types that include images. Overall the benefits of pulling in these changes would seem to outweigh the costs. It would also be great to reduce fragmentation between the different forks, so that future contributions could also be used.
I'll do a bit of testing to see what impact this has if used by the Drupal module.
| ## Usage | ||
|
|
||
| Install via composer: `composer require minicodemonkey/amazon-alexa-php`. | ||
| Install via composer: `composer require froodley/amazon-alexa-php` |
There was a problem hiding this comment.
Use the correct repo name here
|
|
||
| // Fields | ||
|
|
||
| public static $validTypes = [ |
There was a problem hiding this comment.
I don't like this tight coupling on valid request types. It seems like this will break if Amazon adds a new request.
| // Traits | ||
|
|
||
| namespace Alexa\Request; | ||
| use HasPurifier; |
There was a problem hiding this comment.
I agree, using Purifier seems like overkill, but I suppose it could help protect from some theoretical XSS/spoofing attacks.
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "~4.6" | ||
| "phpunit/phpunit": "^5.7.15" |
There was a problem hiding this comment.
This will conflict with Drupal's current dependency on ">=4.8.35 <5". Perhaps use "~4.8" here, instead?
| This library provides provides a convient interface for developing Amazon Alexa Skills for your PHP app. | ||
| This library provides provides a convenient interface for developing Amazon Alexa Skills for your PHP app. | ||
|
|
||
| It represents a breaking change (and is forked) from jakobsuchy/amazon-alexa-php |
There was a problem hiding this comment.
Reword this statement
No description provided.