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

[plugin.video.filmfriend] 1.0.1 #4412

Open
wants to merge 1 commit into
base: matrix
Choose a base branch
from

Conversation

Ingo-FP-Angel
Copy link

Description

  • [Fix] Makes the plugin work again
  • [Feature] Allows to play videos from your personal watchlist

The previous maintainer sarbes seems to have abandoned this plugin (last word from Nov. 2021, see https://forum.kodi.tv/showthread.php?tid=353903&pid=3073503#pid3073503)

Checklist:

  • My code follows the add-on rules and piracy stance of this project.
  • I have read the CONTRIBUTING document
  • Each add-on submission should be a single commit with using the following style: [plugin.video.foo] v1.0.0

@Ingo-FP-Angel Ingo-FP-Angel force-pushed the release_plugin.video.filmfriend_v1.0.1 branch 2 times, most recently from ddfddec to 89890f3 Compare October 31, 2023 14:26
@Ingo-FP-Angel Ingo-FP-Angel force-pushed the release_plugin.video.filmfriend_v1.0.1 branch from 89890f3 to dbba7f6 Compare October 31, 2023 14:27
@basrieter
Copy link
Contributor

@sarbes are you OK with this?

@basrieter basrieter added the Don't merge PR that should not be merged (yet) label Nov 20, 2023
@basrieter
Copy link
Contributor

We still need an answer from @sarbes

@petterreinholdtsen
Copy link

How long is it reasonable to wait for replies from @sarbes before concluding no answer will show up?

@basrieter
Copy link
Contributor

Pinged him again.

@sarbes
Copy link
Member

sarbes commented Jan 23, 2024

Hi there. Sorry for the delay, I was a bit busy in the last few months.

Are you willing to take over the support of the add-on?

@basrieter
Copy link
Contributor

@petterreinholdtsen ping

@petterreinholdtsen
Copy link

petterreinholdtsen commented Feb 3, 2024 via email

@Ingo-FP-Angel
Copy link
Author

Ingo-FP-Angel commented Feb 4, 2024

Are you willing to take over the support of the add-on?

In principle yes.

One small complication: this addon depends on your "script.module.libmediathek4" which needs an additional change for this addon to work. And this PR here just fixes the login issues (also adds the personal watchlist), there are a couple of other things that do not work.

For lack of feedback here I created a fork with a slightly different addon id/name which has the necessary parts of "script.module.libmediathek4" embedded and fixed so using it would work out of the box. That fork is available via the Kodinerds addon repo which allowed me a much faster release/feedback cycle. Your name is still referenced as author and the metadata plus readme mentions where it was forked from.

So right now I'm a little unsure how to proceed. Keep working on the fork, abandon the fork and focus on the official addon repo (needs either fixing "script.module.libmediathek4" or embedding it into this repo), or keep both (the fork for rapid development and only porting stable versions over to the official repo)?

@petterreinholdtsen
Copy link

petterreinholdtsen commented Feb 4, 2024 via email

@sarbes
Copy link
Member

sarbes commented Feb 6, 2024

The idea behind libmediathek was to decouple the content scraper from the Kodi API, so that I could mess with it separately. But besides some private prototypes, I have not used the scrapers in other capacities.

But still, it allowed me to have a single base for my common boiler plate code I use for my add-ons. And in combination by having the scrapers as libraries, I could have one lib powering multiple add-ons. For example, libzdf is used by the ZDF Mediathek, the 3Sat Mediathek and ZDF Tivi.

I don't know how invasive your changes to libmediathek are, but I could update it in my GH repository, so that you could start a PR which I can review. I'm just traveling at the moment, so it would have to wait a bit.

If your changes to LM4 are not detrimental for the other add-ons which use it, it is highly preferred to merge them there to avoid duplicate code we have to review.

@Ingo-FP-Angel
Copy link
Author

In fact somebody else already created a PR with the necessary changes a while ago: sarbes/script.module.libmediathek4#1
The change is pretty minimal I'd say.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Don't merge PR that should not be merged (yet)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants