Bugfix: Correct API version check to support major versions >13 #738
Conversation
The check was not correctly handling major version from 13 on, e.g. the check would have failed for 13.0.
|
@kambala-decapitator, found this while preparing #737. Without this fix the App will lose the support for enhanced playlist/nowplaying visualization (see #543) when the API version is increased at a later point in time. This will impact the users with iOS < 11 as soon as we migrate to Xcode 14. |
|
nice find! although I'd suggest to include #737 in this release to avoid ugly code at once. |
|
I thought of adding this one now, and then add #737 (with further code changes to really make use of the new method at all relevant places) after the release of 1.11. This way we reduce the amount of changes and the risk. |
I agree, let's do this way |
|
but I still think that it's better to create a dedicated helper function for this special >= 12.7 check as it's specifically about "Playlist.Insert and Playlist.Add for recordingid" |
|
|
|
if there're various API checks with dedicated purpose in code, I'd rather create new utils class like |
|
It's only temporary, will all be replaced when #737 gets in. |
|
yes, I understand. I mean, if there're checks that have special meaning just like in this PR, I'd rather have them named somehow instead of throwing comment in every such place. Self-documenting code :) |
|
Got it. |
…laylistSupport Avoids code duplication and speaks for itself when called.
|
You had in mind what I just added with b97d892? If so, I could add a few more named checks. |
|
yes, exactly :) Although I'd call the class |
|
also seems that "if API version not supported, use file path" condition deserves a separate function too - a lot of copy-paste now |
If we add #737 here it will check for both API version and Kodi version.
That's why I added the open action #740. There is more opportunity towards common code. But I still want to keep the changes for 1.11 small. |
|
ok, sounds good |
Description
This PR fixes a problem with a version check which was not correctly handling major versions from 13 on, e.g. the check would have failed for version 13.0 to 13.6. Currently there is no JSON API 13, but the code must be future proof.
Summary for release notes
Bugfix: Correct API version check to support major versions >13