caryyu / jellyfin-plugin-opendouban

Douban metadata provider for Jellyfin
188 stars 15 forks source link

Upgrade to fit the compatibility with 10.8.0 and dotnet 6 #20

Closed LGinC closed 2 years ago

LGinC commented 2 years ago

测试通过,可以在10.8.0-alpha5里使用

caryyu commented 2 years ago

Hi @LGinC ,非常感谢你的贡献;由于该 PR 主要针对兼容 10.8.0-alpha5,能否把该 PR 变更的内容精简为只包括兼容工作内容部分,谢谢;对于注释添加与结构调整再额外开新的 PR,再次感谢你的贡献 :)

caryyu commented 2 years ago

我比较好奇的是目前最低兼容只能做到支持 alpha5 嘛?是否有尝试过 alpha1 - 4 上的测试,你变更后我再拿你最新的代码测试一下 :)

LGinC commented 2 years ago

Hi @LGinC ,非常感谢你的贡献;由于该 PR 主要针对兼容 10.8.0-alpha5,能否把该 PR 变更的内容精简为只包括兼容工作内容部分,谢谢;对于注释添加与结构调整再额外开新的 PR,再次感谢你的贡献 :)

改了。 目测兼容性和Jellyfin那两个nuget包版本有关。如果引用的版本是10.8.0-alpha5应该就只能用于10.8.0-alpha5+

caryyu commented 2 years ago

我的目标是希望可以达到最小兼容 10.8.0-alpha1 ,如果实在不行只能一个一个版本来试了,尽量达到最小兼容;另外,我看见你添加了 CancellationToken 的参数,该参数会和兼容相关吗?

caryyu commented 2 years ago

Hi @LGinC,非常感谢你的贡献,真正兼容的问题产生在 JSON 解析上面,如下代码所示:

https://github.com/caryyu/jellyfin-plugin-opendouban/blob/78a35dd8ab54b9dd5b7ccbf553d64b5a232bbb37/Jellyfin.Plugin.OpenDouban/OddbApiClient.cs?_pjax=%23js-repo-pjax-container%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%2C%20%5Bdata-pjax-container%5D#L58

另外,我已经参考你的部分代码进行了发版,新版本 v2.0.1 将直接兼容 10.8.x-alpha1 及以上 ,如果你之后对代码重构或功能有兴趣,我们统一利用 10.7.x-main 与 10.8.x-main 分别追踪版本升级;你的代码对此次兼容的帮助非常大,再次感谢!我先暂时关闭该 PR 了 :)