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

[BUG] Retrofitting old mycroft skill doesnt create a skill folder and deletes requirements.txt #16

Closed
LordIvanhoe opened this issue Jan 16, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@LordIvanhoe
Copy link

Description

Trying to retrofit mycroft skill, running projen new ovosskill --from "@mikejgray/ovos-skill-projen" --retrofit creates needed folders and files, including requirements.txt
Then, after modifying .projenrc.json and running npx projen, it deletes requirements.txt and does not create skill folder at all. No errors are reported.
Output from npx projen:

vocab folder not found in original skill; skipping.
dialog folder not found in original skill; skipping.
regex folder not found in original skill; skipping.
intents folder not found in original skill; skipping.

Steps to Reproduce

create a folder and put init.py with your skill in it.
inside the folder, run projen new ovosskill --from "@mikejgray/ovos-skill-projen" --retrofit
edit .projenrc.json
run npx projen

Relevant Code

No response

Other Notes

my skill doesnt have localization folders, but i've tried it with the folders too, same result. even if there are folders, projen says no folders, skipping.

@LordIvanhoe LordIvanhoe added the bug Something isn't working label Jan 16, 2024
@LordIvanhoe
Copy link
Author

Weird thing i just realised, if i run npx projen again, it will recreate requirements.txt. Then if i run it AGAIN, it will delete it again. And so on and so forth.

mikejgray added a commit that referenced this issue Jan 17, 2024
We are not bringing this file under projen management, so TextFile is an inappropriate choice
partially fulfills #16
@mikejgray
Copy link
Owner

So I'm seeing two distinct bugs here:

  • requirements.txt popping in and out of existence (fixed now)
  • localization folders are not read

The retrofit flag doesn't have support for old Mycroft skills that had the code in a subdirectory because my understanding is that wasn't supported. Can you tell me more about that use case? If it's something that is somewhat common I'm happy to open a feature request and add support.

Also, would you please share a tree output? Do each of your folders have en-us as a subfolder?

@LordIvanhoe
Copy link
Author

LordIvanhoe commented Jan 17, 2024

So I'm seeing two distinct bugs here:

  • requirements.txt popping in and out of existence (fixed now)
  • localization folders are not read

Plus it doesnt actually create the skill folder.

The retrofit flag doesn't have support for old Mycroft skills that had the code in a subdirectory because my understanding is that wasn't supported. Can you tell me more about that use case? If it's something that is somewhat common I'm happy to open a feature request and add support.

I dont think many people had code inside subfolders, i dont think that would be necessary.

Also, would you please share a tree output? Do each of your folders have en-us as a subfolder?

Here is a fresh test, step by step:
INITIAL FOLDER STRUCT:
initial

After running projen new ovosskill --from "@mikejgray/ovos-skill-projen" --retrofit
after RETROFIT

After running npx projen
after npx projen

And, output from projen that claims that vocab folder doesnt exist.
output from projen

As you can see, in the end, it deleted requirements.txt, and did not create the skill subfolder.

@mikejgray
Copy link
Owner

Not creating the skill folder is intentional - while it is a Python best practice to put the code inside its own folder, it's complex enough that automating it didn't make sense. There are many ways people set up their original Mycroft skills and I don't think I can account for all of them. If you want to move it to a separate folder, do that part manually and add the packageDir variable to your .projenrc.json file, then run npx projen.

As for the rest, it looks like you're still running an old version of the code. You should be able to clear the package manager cache:

npm cache clean --force
yarn cache clean

And try again, so it will pull the latest version this time.

To confirm, did you add the following to your ~/.npmrc file? Create one if it doesn't exist.

@mikejgray:registry=https://npm.pkg.github.com

@mikejgray
Copy link
Owner

I spoke with @LordIvanhoe directly on Matrix and worked through some issues loading the latest version of this package. Added the outcome to the README.md file to avoid issues like this in future. Once we've confirmed the new code does what he expects, we can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants