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

Split mycroft.skills.core #2221

Merged
merged 4 commits into from Jul 26, 2019

Conversation

@forslund
Copy link
Member

commented Jul 19, 2019

Description

  • MycroftSkill and related core functions to mycroft_skill.py
  • FallbackSkill to fallback_skill.py
  • Add important classes decorators and functions to init.py

core.py retains the same members as previously by means of imports to
retain backwards compatibility. When most of the available skills starts
using this new structure properly.

This makes some of the code easier to view. the mycroft_skill.py still weighs in at ~1400 LOC and we should work at splitting that out in more logical components. I have a suggestion as WIP using a couple of member classes, but the initial split-up is a good first step.

How to test

Make sure that existing skills load and executes like before.

Contributor license agreement signed?

CLA [ Yes ]

@forslund forslund force-pushed the forslund:refactor/split-skills-core branch 2 times, most recently from 1082081 to 7c1ffe2 Jul 19, 2019
@penrods

This comment has been minimized.

Copy link
Member

commented Jul 20, 2019

I definitely like this reorg!

mycroft/skills/fallback_skill.py Outdated Show resolved Hide resolved
mycroft/skills/fallback_skill.py Outdated Show resolved Hide resolved
mycroft/enclosure/gui.py Outdated Show resolved Hide resolved
mycroft/enclosure/gui.py Show resolved Hide resolved
mycroft/enclosure/gui.py Show resolved Hide resolved
mycroft/skills/__init__.py Show resolved Hide resolved
mycroft/skills/fallback_skill.py Outdated Show resolved Hide resolved
- MycroftSkill and related core functions to mycroft_skill.py
- FallbackSkill to fallback_skill.py
- Add important classes decorators and functions to __init__.py
- move SkillGUI class to the enclosure along with the other enclosure
  interfaces

core.py retains the same members as previously by means of imports to
retain backwards compatibility. When most of the available skills starts
using this new structure properly
@forslund forslund force-pushed the forslund:refactor/split-skills-core branch from 7c1ffe2 to b7b5e9b Jul 22, 2019
@forslund

This comment has been minimized.

Copy link
Member Author

commented Jul 22, 2019

My original intent was just to move things around into new locations but I can do some additional refactoring as you've suggested.

@forslund forslund changed the title Refactor mycroft.skills.core Split mycroft.skills.core Jul 22, 2019
forslund added 3 commits Jul 22, 2019
Update docstrings of the files affected by this reoriganization.
- PEP 257
- Add missing
@forslund forslund merged commit 5eba242 into MycroftAI:dev Jul 26, 2019
3 checks passed
3 checks passed
:-) Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+1.06%) to 51.181%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.