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

Container Identifiers #58

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

Conversation

dimitribouniol
Copy link

To help users who need to configure a production and development container, added two new containerIDs: production and development. Also added documentation and clarified how the default ID is used, which isn't always the default depending on how use is called.

Note: Since this is new, I went with development instead of sandbox here because all of Apple's documentation refers to it as the development environment. We should probably add .development and deprecate .sandbox in APNSwift as well.

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

One minor addition

/// You must configure at lease one client in order to send notifications to devices. If you plan on supporting both development builds (ie. run from Xcode) and released builds (ie. TestFlight/App Store), you must configure at least two configurations:
///
/// ```swift
/// let apnsKey = ... // The .p8 file as a string.
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a note here to suggest passing it in as an environment variable and not hardcoding it in the source code

Copy link
Author

Choose a reason for hiding this comment

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

Good callout, I’ll add a section for security considerations.

Copy link
Author

Choose a reason for hiding this comment

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

Updated!

@0xTim 0xTim added the semver-minor Contains new APIs label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor Contains new APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants