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

Void kampff skill source check #2714

Open
forslund opened this issue Oct 2, 2020 · 6 comments
Open

Void kampff skill source check #2714

forslund opened this issue Oct 2, 2020 · 6 comments
Labels
help wanted Status: For discussion Feature proposal in development. Community input and discussion is invited. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.

Comments

@forslund
Copy link
Collaborator

forslund commented Oct 2, 2020

The actual checking of skill source isn't implemented due to an on-going discussion (at least it was back when these were implemented), determining the format of the skill name.

skill-id is what is reported in the speak meta data but it isn't quite friendly for writing the behave files. So some sort of translation is needed skill-id -> pretty name. I'll add an issue reflecting this.

Suggested lookup:

  • skill name, taken from speech metadata field
  • pretty name table, allowing use of names displayed on the skill page. A custom table translating known names to pretty names, this can be generated from skill meta data, but won't exist for unsubmitted skills...possibly for these it can be generated from the readme?
  • skill folder names
@forslund forslund added the Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap. label Oct 2, 2020
@damorosodaragona
Copy link

damorosodaragona commented Oct 3, 2020

@forslund Just to understand, what do you mean with "pretty name" and what with "skill-id"?
Can you give me an real example?
I want be sure that i'm following you.

@forslund
Copy link
Collaborator Author

forslund commented Oct 3, 2020

I was wrong about skill id. The system uses the self.name as skill which can be provided explicitly to skill or it uses the skill class name as name.

"Pretty name" is something like "Mycroft Weather" or "The weather skill" something that works well in natural text. The "pretty name" the skills meta uses is the skill name shown on the skill page of home.mycroft.ai.

@krisgesling, @chrisveilleux, @dschweppe might have opinions on vwhat is a "pretty name" and the implementation as well.

@damorosodaragona
Copy link

damorosodaragona commented Oct 4, 2020

Oh ok.
I'm new and i'm understanding how works mycroft day by day, but i didn't see anything in the metadata about skill-id, for this i asked.

In my opinion is better and more useful use the skill name (self.name) in the test.
If i write a skill, i have to know it and is also more simple remember the name class of my skill (self.name)
Using another name to referer about the same skill, the pretty name as you called it, in my opinion could be cause of more mistakes and more confusion.

@forslund
Copy link
Collaborator Author

forslund commented Oct 4, 2020

Updated the initial issue with the correction

@krisgesling
Copy link
Contributor

I think this has improved as we removed the name field from Skill settings, but there's some further work needed. Eg for the Hello World Skill:
self.name => "HelloWorldSkill"
Selene display (from README title) => "Hello World"
file system (from the repo slug I think?) => "mycroft-hello-world.mycroftai"

I wouldn't be surprised if there's somewhere else that the Skill name is used in a slightly different way too. If we can cut this down to the first two it would be good. Though may need to consider the case of a Skill not having a readme.

Part of the idea behind Behavioural based tests is that they are human readable (and writable), including by non-devs. So personally I think it's worth supporting both the "pretty name" and the "class name" (self.name) for this.

@krisgesling krisgesling added help wanted Status: For discussion Feature proposal in development. Community input and discussion is invited. labels Oct 5, 2020
@forslund
Copy link
Collaborator Author

forslund commented Oct 5, 2020

Indeed, to make self.name more readable/writable the comparison can also be made after running the name through camel_case_split(), HelloWorldSkill -> Hello World Skill.

I'll add the skill foldername as you suggest to the issue description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Status: For discussion Feature proposal in development. Community input and discussion is invited. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.
Projects
None yet
Development

No branches or pull requests

3 participants