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

Remove docs encouraging Skill config via mycroft.conf #2936

Merged
merged 1 commit into from Jun 28, 2021

Conversation

krisgesling
Copy link
Contributor

Description

Removes an outdated section of the main README that encouraged the use of mycroft.conf for Skill configuration.

The example used was also outdated as you cannot currently configure the mycroft-weather Skill to use a local API key.

Raised in MycroftAI/skill-weather#170

How to test

Markdown only

Contributor license agreement signed?

  • CLA

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

Voight Kampff Integration Test Succeeded (Results)

@codecov-commenter
Copy link

Codecov Report

Merging #2936 (cfe03a4) into dev (8fa6508) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #2936      +/-   ##
==========================================
- Coverage   52.62%   52.61%   -0.01%     
==========================================
  Files         123      123              
  Lines       11016    11016              
==========================================
- Hits         5797     5796       -1     
- Misses       5219     5220       +1     
Impacted Files Coverage Δ
mycroft/client/speech/listener.py 48.04% <0.00%> (-0.36%) ⬇️
mycroft/tts/cache.py 83.12% <0.00%> (ø)

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 8fa6508...cfe03a4. Read the comment docs.


- [STT API, Google STT, Google Cloud Speech](http://www.chromium.org/developers/how-tos/api-keys)
- [A range of STT services](https://mycroft-ai.gitbook.io/docs/using-mycroft-ai/customizations/stt-engine) are available for use with Mycroft.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the intended indentation level? It looks a bit weird in the preview

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it was. It does look a little weird, but it's part of the point above...
I'm just going to go with it for the moment.
The whole README could probably do with a refresh...

Thanks for the eagle eyes though :)

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.

Looks good IMO, there was an indentation I wasn't sure if it was intended or not. Assuming it was I approve

@JarbasAl
Copy link
Contributor

Makes sense to me, the individual key setup is better documented in the individual skills.

In here it would have a tendency to get outdated, another advantage is that the readme changes become part of the individual skill git history

@krisgesling krisgesling merged commit 2716514 into dev Jun 28, 2021
@krisgesling krisgesling deleted the bugfix/readme-skill-config branch June 28, 2021 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants