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 ourterms of serviceand privacy statement.We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[prebuild-config][iOS] remove automatic APNS entitlement" #30280

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

douglowder
Copy link
Contributor

@douglowder douglowder commented Jul 8, 2024

This reverts#27924.

Why

The above change was requested by some customers who did not need notifications in their iOS app and did not want the APNS entitlement automatically added.

Unfortunately, this resulted in most customers seeing warnings from Apple on submission of their apps to the App Store. (Seehttps://github /expo/fyi/blob/main/apns-entitlement-sdk-51.mdfor details). The warnings are simply informational, and do not block acceptance of apps to the App Store.

For the convenience of our customers, we have made the decision to revert the above change. We will be providing documentation shortly for the steps needed to actually remove the warning for apps without the APNS entitlement.

Test Plan

CI should pass.

Checklist

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jul 8, 2024
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jul 8, 2024
@brentvatne
Copy link
Member

i thought about this a bit more, and one concern that i have now is that this change will break non-interactive builds for folks who have created a build in this cycle without the APNS entitlement configuration, and therefore without the capability enabled. my understanding is that if we set the entitlement and the capability isn't set on the provisioning profile, then the build will fail. so, github triggered builds or other ci builds for these folks would fail suddenly. so, i think i'm reluctant to roll this out mid-cycle after all.@douglowder- if you can write out a notion doc that outlines the problem here in detail, then we can see if the sdk team has any other thoughts on how we can handle this in expo-modules-core

@douglowder
Copy link
Contributor Author

@brentvatneI agree that this should not be rolled out midcycle, it should wait until SDK 52 and be listed in the changelog for the release. The problem is already outlined in detail in the FYI article in the description, and inENG-12661.

I'll do some testing to see if adding back the entitlement breaks the app build if the provisioning profile is not also updated.

@douglowder
Copy link
Contributor Author

@brentvatnefor you and other Expo engineers, I've added aNotion docdetailing the APNS entitlement issue, and summarizing my recommendations.

@douglowder douglowder force-pushed the doug/eng-12661-notificationsiosdocs-removing-notifications-uiappdelegate branch from a9eba69 to 11e93c7 Compare August 1, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint compatible bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants