8000 Fix replacement of existing Skill loader in queue by krisgesling · Pull Request #2899 · MycroftAI/mycroft-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Sep 8, 2024. It is now read-only.

Fix replacement of existing Skill loader in queue #2899

Merged
merged 1 commit into from
May 7, 2021

Conversation

krisgesling
Copy link
Contributor

Description

Fixes a typo in the Skill loader queue methods. It was evaluating rather than assigning the output of a list comprehension.

This would prevent the existing Skill loader from being removed from the queue but still append the new one.

Thanks to @JarbasAI for spotting this!

How to test

Extended existing unit tests to check for this scenario.

Contributor license agreement signed?

@krisgesling krisgesling added the Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained. label May 6, 2021
@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 requested a review from chrisveilleux May 6, 2021 02:57
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.

NIce find and nice fix. Great to have the unittest expanded to cover this!

@krisgesling krisgesling merged commit 09f24e1 into dev May 7, 2021
@krisgesling krisgesling deleted the bugfix/remove-existing-loader branch May 7, 2021 21:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Type: Bug - quick Bug fixes that are quick to review and the implications of the change are clear and contained.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0