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

feat: current practices #8

Merged
merged 6 commits into from
May 17, 2023
Merged

Conversation

mikejgray
Copy link

Updates the hello-world skill to use the latest imports and practices as I understand them from the Matrix chat and current documentation.

Also adds a generic Python .gitignore and thus deletes the dist/ and build/ folders and all their extras.

__init__.py Show resolved Hide resolved
__init__.py Outdated Show resolved Hide resolved
@JarbasAl
Copy link
Member

JarbasAl commented May 17, 2023

im ok with merging as is but left a couple comments

def stop could also be removed if not used, or should have a comment. stop method should return True if it stopped something or False (or None) otherwise

def create_skill is also no longer needed

@JarbasAl JarbasAl added the enhancement New feature or request label May 17, 2023
@mikejgray
Copy link
Author

im ok with merging as is but left a couple comments

def stop could also be removed if not used, or should have a comment. stop method should return True if it stopped something or False (or None) otherwise

I'll make the updates from the comments

@mikejgray
Copy link
Author

def create_skill is also no longer needed

Can you elaborate more on this?

@JarbasAl
Copy link
Member

def create_skill is also no longer needed

Can you elaborate more on this?

the method at bottom of the file is no longer needed, now the skill class is auto detected + also provided in setup.py

that function at end of the file is just dead code more or less, we init skill classes directly whenever possible

the only use for this method would be if you had multiple skill classes in a single package and wanted to do add some logic in there to decide what to load

@mikejgray mikejgray requested a review from JarbasAl May 17, 2023 02:27
@JarbasAl JarbasAl merged commit 2bfb76b into OpenVoiceOS:dev May 17, 2023
@mikejgray mikejgray deleted the FEAT_CurrentPractices branch May 17, 2023 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants