Closed DFranzen closed 6 years ago
Sorry for the multiple pull requests, I could not figure out how to invoke travis without resubmitting.
Hi @DFranzen. Thank you for the PR, some great ideas there!
Generally it's better if you raise an issue first to allow discussion around direction. I could help out with creating them if you want. It seems like there are two (or three) logical changes here:
SetVolumeIntent
support (edit: I've found it: #12)If you could split the smaller one(s) out (i.e. a separate PR from master
), it would make merging easier as they can usually go in (first) without much fuss.
For the internationalisation, this could be using a Python internationalisation framework. They're a bit clunky sometimes but gettext
(we use it on Quod Libet for the 30+ languages we support and it works well enough. It also solves problems that aren't solved easily - e.g. plural forms.
Ideally there should be some new unit tests around any new code - squeeze-alexa
is a highly tested codebase as I'm a big believer in unit / integration testing.
Alright. Fair enough. I will start a discussion in two separate issues for the SetVolumeIntent and the Internationalisation.
Should I/you close this pull request until we reach a verdict?
BTW: I did not mention it directly, but I have the skill up and running on a Raspberry pi
Should I/you close this pull request until we reach a verdict?
OK yes - let's shut this one for now
I have refactored the speech and text responses for Alexa, to have all the snippets in an external file. This way it was easy to add German responses and other languages should be straight forward in the future. The used language can be selected in the settings file The result should feel identical for the English Version, while a new German version of the interaction is present.
I have also implemented the missing SetVolumeIntent, which was in the schema and utterances but missing in code.