New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update CommonPlaySkill and CommonQuerySkill to pass messages #2876
base: dev
Are you sure you want to change the base?
Conversation
Hello @NeonDaniel! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-03-31 20:29:43 UTC |
Hello, @NeonDaniel, thank you for helping with the Mycroft project! We welcome everyone To protect yourself, the project, and users of Mycroft technologies we require Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank |
Voight Kampff Integration Test Failed (Results). |
1 similar comment
Voight Kampff Integration Test Failed (Results). |
Fix common_query_test
Voight Kampff Integration Test Failed (Results). |
Hello, @NeonDaniel, thank you for helping with the Mycroft project! We welcome everyone To protect yourself, the project, and users of Mycroft technologies we require Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank |
Description
Fixes #2875
Similar to #2813, this updates the CPS and CQS abstract methods to expect Messages instead of Strings. Inspecting the method signatures provides backwards compatibitliy.
How to test
This should work with all existing skills as-is. New functionality can be tested by modifying
match_query_phrase
andaction
overriding methods to match the new signatureContributor license agreement signed?
CLA [x ] (Whether you have signed a CLA - Contributor Licensing Agreement