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

Tvdb v4 migration #93

Merged
merged 36 commits into from
Feb 11, 2024
Merged

Tvdb v4 migration #93

merged 36 commits into from
Feb 11, 2024

Conversation

scampower3
Copy link
Member

@scampower3 scampower3 commented Aug 6, 2023

Tvdb v4 migation in progress. Will set on draft till its done.
Some code commented out needs some clarification.

Fixes #47
Fixes #100

Using this fork of TVdbSharper: https://github.com/scampower3/TvDbSharper
Switched to: https://github.com/crobibero/tvdb-sdk-csharp

@scampower3 scampower3 marked this pull request as ready for review August 9, 2023 14:37
@scampower3
Copy link
Member Author

Should be working without any bugs now.

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initial pass

Jellyfin.Plugin.Tvdb/TvdbUtils.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
@scampower3
Copy link
Member Author

Should I include the TVSharper DLL inside the PR? Or is it better if you build the DLL from my TvDbSharper fork?

@crobibero
Copy link
Member

Should I include the TVSharper DLL inside the PR? Or is it better if you build the DLL from my TvDbSharper fork?

I don't think so- I would say this PR is blocked until upstream merges your changes (HristoKolev/TvDbSharper#39)

Alternatively we can migrate to the sdk I have generating (https://github.com/crobibero/tvdb-sdk-csharp)

@scampower3
Copy link
Member Author

Hmm, I doubt he will merge my changes anytime soon since the last commit for that project is in 2022. Maybe we can keep this as an unoffical tvdb plugin until the changes are merged or we migrate to the sdk?

@scampower3 scampower3 marked this pull request as draft December 2, 2023 14:36
Comment on lines +232 to +239
if (dvdInfo is null)
{
item.IndexNumber = episode.Number;
}
else
{
item.IndexNumber = Convert.ToInt32(dvdInfo.Number, CultureInfo.InvariantCulture);
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Comment on lines +65 to +72
if (string.IsNullOrWhiteSpace(parts[1]))
{
threeletterNames = new[] { parts[0] };
}
else
{
threeletterNames = new[] { parts[0], parts[1] };
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
Jellyfin.Plugin.Tvdb/TvdbClientManager.cs Fixed Show fixed Hide fixed
@luevano
Copy link

luevano commented Dec 20, 2023

Hi @scampower3 any updates on this? wondering if this is still being worked on, or is it going to be fixed by another PR

@scampower3
Copy link
Member Author

scampower3 commented Dec 21, 2023

Hi @scampower3 any updates on this? wondering if this is still being worked on, or is it going to be fixed by another PR

Hey, this is PR isnt abandoned. This migration is more or less done last I checked. However, it is dependent on the changes made here. This PR is probably blocked till the schema on TVDB api site is fixed.

@scampower3 scampower3 marked this pull request as ready for review February 11, 2024 15:19
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeImageProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbEpisodeProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbSeriesProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbSeriesProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbSeriesProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbSeriesProvider.cs Outdated Show resolved Hide resolved
Jellyfin.Plugin.Tvdb/Providers/TvdbSeriesProvider.cs Outdated Show resolved Hide resolved
@crobibero crobibero added the major-feature This PR introduces a major new feature label Feb 11, 2024
@crobibero
Copy link
Member

Thank you very much for going through this adventure!

@crobibero crobibero merged commit 1b6930b into jellyfin:master Feb 11, 2024
3 checks passed
@scampower3 scampower3 deleted the TVDB-v4 branch February 12, 2024 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-feature This PR introduces a major new feature
Projects
None yet
3 participants