Skip to content
This repository was archived by the owner on Sep 8, 2024. It is now read-only.

Add Common Play to active Skills when invoked #2900

Merged
merged 1 commit into from
Jun 29, 2021
Merged

Add Common Play to active Skills when invoked #2900

merged 1 commit into from
Jun 29, 2021

Conversation

krisgesling
Copy link
Contributor

Description

Emit message on bus to add Common Play to the active Skills list when it is invoked.

This prevented converse() methods in Skills using the Common Play Framework from being registered as active, hence they would not work as expected.

Note: Companion PR to #2897 for the dev branch of core

How to test

Use the fix_no_stop branch of the News Skill

Contributor license agreement signed?

@pep8speaks
Copy link

pep8speaks commented May 6, 2021 •
edited
Loading

Hello @krisgesling! 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-05-07 04:55:53 UTC

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label May 6, 2021
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling force-pushed the bugfix/common-play-active branch from 12dfbe4 to a968909 Compare May 6, 2021 11:07
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

JarbasAl
JarbasAl reviewed May 6, 2021
View reviewed changes
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling force-pushed the bugfix/common-play-active branch from a968909 to 249e352 Compare May 7, 2021 04:55
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

2 similar comments
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results).
Mycroft logs are also available: skills.log, audio.log, voice.log, bus.log, enclosure.log

forslund
forslund approved these changes Jun 22, 2021
View reviewed changes
Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since @JarbasAI's comment has been resolved, I think this can be merged :)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@krisgesling krisgesling merged commit 0cbd040 into dev Jun 29, 2021
@krisgesling krisgesling deleted the bugfix/common-play-active branch June 29, 2021 11:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@JarbasAl JarbasAl JarbasAl left review comments

@forslund forslund forslund approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@krisgesling @pep8speaks @devops-mycroft @forslund @JarbasAl @ken-mycroft