Skip to content
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

Don't kill global events when shutting down skill #2296

Merged
merged 1 commit into from Sep 10, 2019

Conversation

@forslund
Copy link
Member

commented Sep 10, 2019

Description

During skill shutdown remove_all_handlers() was called on all registered
events. This would unregister all global events, such as settings
updating, stop for all skills.

Now the remove only will remove the instance connected to the skills
method.

This is a likely cause for the failure to stopping the news playback.

How to test

Start skills, trigger a reload of a single skill. Ensure that the news skill can be stopped still.

Contributor license agreement signed?

CLA [ Yes ]

During skill shutdown remove_all_handlers() was called on all registered
events. This would unregister all global events, such as settings
updating, stop for all skills.

Now the remove only will remove the instance connected to the skills
method.
@forslund forslund requested a review from davidwagnerkc Sep 10, 2019
Copy link
Contributor

left a comment

Tested 👍

@forslund forslund merged commit ca37d18 into MycroftAI:dev Sep 10, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 53.246%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.