MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.

https://mycroft.ai

Apache License 2.0

6.48k stars 1.27k forks source link

Voigt kampff: Bugfix dialogfile from sentence #3068

Closed forslund closed 2 years ago

forslund commented 2 years ago

Description

Fix two issues with "skill" should reply with "sentence" behave step.

locale folder was not checked for dialog files
A redundant (?) regex step caused issues if there were no keywords

Should resolve #3067

How to test

Ensure VK tests still works, see test case in #3067 for previously failing test.

Contributor license agreement signed?

CLA [ x ]

devvmh commented 2 years ago

i've applied https://github.com/forslund/mycroft-core/commit/03ef7f4b219533507f016c64a45487bc827ec78c and https://github.com/forslund/mycroft-core/commit/825b3879a51c15513e069e61872266bfa55ecd99 locally but don't have it working yet - i must need to refresh a cache or something.

assuming it works, i would love to see this backported to master since i'm not sure i'm ready to move to dev branch... but i can struggle along with cherry-picking it otherwise

pep8speaks commented 2 years ago

Hello @forslund! 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! :beers:

Comment last updated at 2022-02-23 06:01:56 UTC

codecov-commenter commented 2 years ago

Codecov Report

Merging #3068 (f8aa77c) into dev (3d963ce) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #3068   +/-   ##
=======================================
  Coverage   53.08%   53.08%           
=======================================
  Files         123      123           
  Lines       11192    11192           
=======================================
  Hits         5941     5941           
  Misses       5251     5251

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3d963ce...f8aa77c. Read the comment docs.

krisgesling commented 2 years ago

Thanks @devvmh your review is very helpful!

I've been away having a baby and covid... but hopefully all our CI tests are happy now and we can merge it in :tada:

devops-mycroft commented 2 years ago

Voight Kampff Integration Test Succeeded (Results)