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 deployment target to match React Native (8.0) #91

Merged
merged 1 commit into from
Apr 19, 2017

Conversation

ndbroadbent
Copy link
Contributor

I figured this out when the build was crashing on my simulator running iOS 8.4. With help from this comment on SO.

@alvaromb
Copy link
Collaborator

Hi @ndbroadbent!

Have you checked the implications of this change? Are the ads SDK supported for iOS 8.0?

@ndbroadbent
Copy link
Contributor Author

Hi, I did test this on the iOS Simulator running iOS 8.4, and everything seemed to work fine.

I think the SDK supports iOS 7 and higher: https://firebase.google.com/docs/admob/ios/download

@philipheinser
Copy link

Also works for us with the target set to 8.0.

@alvaromb
Copy link
Collaborator

alvaromb commented Mar 9, 2017

So, can we safely set the target to 8.0?

@alvaromb
Copy link
Collaborator

Ping @ndbroadbent @philipheinser

@ndbroadbent
Copy link
Contributor Author

Hi, yes, should be totally safe to merge this. Apps cannot work on iOS 8 without this change.

@alvaromb
Copy link
Collaborator

Ok, @ndbroadbent, thanks for your quick feedback.

I'm in the middle of something, it's ok for you if we release this later this week?

@alvaromb alvaromb self-assigned this Mar 21, 2017
@ndbroadbent
Copy link
Contributor Author

Yes, no problem :) I am already just using a fork with my own changes, so I'm not in any hurry

@ndbroadbent
Copy link
Contributor Author

@alvaromb sorry, just another ping on this. I just rebased to the latest commit on master

@alvaromb alvaromb merged commit a9d27c0 into sbugert:master Apr 19, 2017
aparedes pushed a commit to aparedes/react-native-admob that referenced this pull request Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants