search

MycroftAI / mycroft-core

Mycroft Core, the Mycroft Artificial Intelligence platform.
https://mycroft.ai
Apache License 2.0
6.51k stars 1.27k forks source link

[mycroft-config] Fix issue #2991 #3015

Closed goldyfruit closed 2 years ago

goldyfruit commented 2 years ago

Description

mycroft-config set command still uses old non-existant mycroft.conf

How to test

Execute mycroft-config set

Contributor license agreement signed?

CLA [ Yes ]

devops-mycroft commented 2 years ago

Voight Kampff Integration Test Succeeded (Results)

devs-mycroft commented 2 years ago

Hello, @goldyfruit, thank you for helping with the Mycroft project! We welcome everyone into the community and greatly appreciate your help as we work to build an AI for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require a Contributor Licensing Agreement (CLA) before accepting any code contribution. This agreement makes it crystal clear that along with your code you are offering a license to use it within the confines of this project. You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank you!

forslund commented 2 years ago

With the minor change suggested in the comment this seems to work quite well. Thanks for tackling this issue!

codecov-commenter commented 2 years ago

Codecov Report

Merging #3015 (35d7a68) into dev (3495acf) will increase coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head 35d7a68 differs from pull request most recent head 782c174. Consider uploading reports for the commit 782c174 to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##              dev    #3015   +/-   ##
=======================================
  Coverage   53.04%   53.04%           
=======================================
  Files         123      123           
  Lines       11167    11167           
=======================================
+ Hits         5923     5924    +1     
+ Misses       5244     5243    -1
Impacted Files Coverage Δ
mycroft/client/speech/listener.py 48.39% <0.00%> (+0.35%) :arrow_up:

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 3495acf...782c174. Read the comment docs.

forslund commented 2 years ago

I've approved the change, will leave it to the core team to merge.

A question (and this is not only this change but seems to be common throughout the file):

Should $_config_file be wrapped in quotes (") in case there is a space in the path to the folder?

devs-mycroft commented 2 years ago

Hello, @goldyfruit, thank you for helping with the Mycroft project! We welcome everyone into the community and greatly appreciate your help as we work to build an AI for Everyone.

To protect yourself, the project, and users of Mycroft technologies we require a Contributor Licensing Agreement (CLA) before accepting any code contribution. This agreement makes it crystal clear that along with your code you are offering a license to use it within the confines of this project. You retain ownership of the code, this is just a license.

Please visit https://mycroft.ai/cla to initiate this one-time signing. Thank you!

goldyfruit commented 2 years ago

I already have the CLA

ChanceNCounter commented 2 years ago

Yeah, two robots are offering differing opinions about that.

goldyfruit commented 2 years ago

recheck

krisgesling commented 2 years ago

Excellent, thanks for this!

PureTryOut commented 2 years ago

Should $_config_file be wrapped in quotes (") in case there is a space in the path to the folder?

@forslund, probably yes. These things are why you should always run shellcheck on scripts so it warns you about common pitfalls like that. The CI should probably run it too