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.

Sign up for GitHub

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

Jump to bottom

Replace kodiwrapper with kodiutils #575

Merged
merged 1 commit into from
Nov 15, 2019
Merged

Replace kodiwrapper with kodiutils #575

merged 1 commit into from
Nov 15, 2019

Conversation

dagwieers
Copy link
Collaborator

@dagwieers dagwieers commented Nov 15, 2019 •
edited
Loading

This is an attempt to simplify the addon.

This PR includes:

  • Removing the KodiWrapper class in favor of discrete functions (kodiutils)
  • Rename some kodiutils functions to shorter variants
  • Selective imports from kodiutils and json
  • Make more use of has_addon()
  • Remove unused code (resumepoints/favorites invalidate_caches)
  • Instantiate Addon() every time, don't cache it (this fixes Changes to settings do not take effect immediately #560)

This fixes #560

@dagwieers dagwieers added the enhancement New feature or request label Nov 15, 2019
@dagwieers dagwieers added this to the v2.3.0 milestone Nov 15, 2019
@dagwieers dagwieers force-pushed the kodiutils branch 8 times, most recently from a281ab9 to 7b936b2 Compare November 15, 2019 11:39
@dagwieers

This comment has been minimized.

Sign in to view
@dagwieers dagwieers force-pushed the kodiutils branch 2 times, most recently from f2b90b3 to df69744 Compare November 15, 2019 12:27
@dagwieers dagwieers mentioned this pull request Nov 15, 2019
Closed
@dagwieers
Replace kodiwrapper with kodiutils
4dd1d9d 
This is an attempt to simplify the addon and make changes to setting
take immediate effect.
@mediaminister
Copy link
Collaborator

mediaminister commented Nov 15, 2019 •
edited
Loading

I just did a quick test and I can confirm this fixes #560.
You can merge this already, I will test the master branch more thoroughly later this weekend.

@dagwieers
Copy link
Collaborator Author

dagwieers commented Nov 15, 2019 •
edited
Loading

Do you agree with the removal of KodiWrapper as well ?
I think it will help with reusing that library (or pieces of it) elsewhere as well.

@mediaminister
Copy link
Collaborator

Yes, I totally agree with removing KodiWrapper. I think simplifying the addon structure is very much needed.

@dagwieers dagwieers merged commit 0b9b130 into add-ons:master Nov 15, 2019
@dagwieers dagwieers mentioned this pull request Nov 29, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mediaminister mediaminister mediaminister approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v2.3.0
Development

Successfully merging this pull request may close these issues.

Changes to settings do not take effect immediately
2 participants
@dagwieers @mediaminister