search

jellyfin / jellyfin-plugin-nextpvr

https://jellyfin.org
MIT License
25 stars 9 forks source link

Create a new plugin for NextPVR v5 #11

Closed emveepee closed 3 years ago

emveepee commented 3 years ago

This is the v5 plugin for v5 of NextPVR. I need a new location to merge it with since it should not go in master which is for v4 of NextPVR

Nickbert7 commented 3 years ago

I think it would make sense to create a new repo and transfer it to the organization. (If I remember correct how it was done in the past with other apps and as mentioned here🤔)

I assume this is the plugin you have in the NPVR forum, so then I can confirm that it is working fine with the latest JF stable and NPVR v5 and is ready to use. But as I'm no developer I'm not able to review the code...

Also as mentioned in the Matrix chat it makes sense to keep the old and new plugin separated.

emveepee commented 3 years ago

I assume this is the plugin you have in the NPVR forum, so then I can confirm that it is working fine with the latest JF stable and NPVR v5 and is ready to use.

With the version you used since 10.6 in August I did mess up configPage.html with a bad rebase of PR10, so for new users it would have only worked on localhost:8866 otherwise the code itself was the same. I did post that update today which is a compile of this PR.

joshuaboniface commented 3 years ago

Sorry for the delay @emveepee.

To avoid needing a new repo, I'm going to replace the current master branch with this. If it's all ready to go, then we can merge it in!

joshuaboniface commented 3 years ago

We'll want to avoid renaming the internal directory from "NextPvr" to "Source". That just complicates things and all the other plugins have the internal project dir named after the real name. Are you able to fix that up? That should fix the merge conflicts too.

joshuaboniface commented 3 years ago

I've pushed the existing code to the branch v4 so we can safely replace whatever is in master with this.

emveepee commented 3 years ago

We'll want to avoid renaming the internal directory from "NextPvr" to "Source". That just complicates things and all the other plugins have the internal project dir named after the real name. Are you able to fix that up? That should fix the merge conflicts too.

I want to keep it Source, src or buildsrc like many other repos. This is not NextPVR that is the backend program

joshuaboniface commented 3 years ago

All of our other plugins use the name of the plugin and I'd liked to ensure that remains consistent. This is also seemingly the source of these merge conflicts.

emveepee commented 3 years ago

All of our other plugins use the name of the plugin and I'd liked to ensure that remains consistent. This is also seemingly the source of these merge conflicts.

If there is a standard it is often Jellyfin.Plugin.. more often than not but I would prefer that even if it is redundant. The source of the conflict is more likely how logger changed after my original pull.

joshuaboniface commented 3 years ago

Sure I'm OK with that.

If you can rename that I can manually merge this on the CLI to override the merge conflicts.

emveepee commented 3 years ago

Sure I'm OK with that.

If you can rename that I can manually merge this on the CLI to override the merge conflicts.

Sounds good I will have a look at the conflicts too i thought I'd submit what users were testing with. I assume it is better than what i quickly did to get it to work

joshuaboniface commented 3 years ago

OK in that case yea, if you're able to fix them up entirely that's even better, given how extensive your rewrite is.

emveepee commented 3 years ago

I am closing this PR #12 takes precendence