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

fix(tvos): fix tv os build and sample #3785

Merged

Conversation

freeboub
Copy link
Collaborator

Summary

Fix build issue on tvOS and ensure sample is working

Motivation

Fix: #3779

Changes

Conditionnal build patch

Test plan

build and run sample tvOS app

@freeboub freeboub requested a review from KrzysztofMoch May 16, 2024 10:15
Comment on lines +1048 to +1050
#if !os(tvOS)
viewController.updatesNowPlayingInfoCenter = false
#endif
Copy link
Member

Choose a reason for hiding this comment

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

I don't like it... docs say it should work... I will look into this - give me time

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also don't really like these #if ...
I agree doc say it is supported, but when I go to definition from xcode, It doesn't say it is support on tvOS ...

Copy link
Member

Choose a reason for hiding this comment

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

So I have double checked that and it's it as you say - strange
Let's merge this

@@ -9,7 +9,7 @@ production = ENV["PRODUCTION"] == "1"

target 'exampletvOS' do

platform :ios, min_ios_version_supported
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@KrzysztofMoch I think we need to document this point also !
Would that fix be better ? #3737 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I am also thinking about adding this info to README (that min ios version is 13) and to upgrade helper

@KrzysztofMoch KrzysztofMoch merged commit cd42dd7 into TheWidlarzGroup:master May 20, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants