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

Remove App Check Core Interop SDK #11595

Merged
merged 4 commits into from
Jul 24, 2023
Merged

Conversation

andrewheard
Copy link
Contributor

The ObjC Protocol-only interop SDK is not needed by our integrators.

#no-changelog

@andrewheard andrewheard marked this pull request as ready for review July 24, 2023 20:08
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up.

LGTM after addressing the typedef comment.


NS_ASSUME_NONNULL_BEGIN

NS_SWIFT_NAME(AppCheckCoreTokenHandler)
Copy link
Member

Choose a reason for hiding this comment

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

typedefs should be unavailable for Swift like - https://github.com/firebase/firebase-ios-sdk/blob/master/FirebaseAuth/Sources/Public/FirebaseAuth/FIRAuth.h#L38 and should be avoided in public headers for ObjC if reasonable since they'll add tech debt to the API surface when we convert to Swift.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads up on the Swift interop! I think this one was reasonable to remove from the header.

Removed the GACAppCheckTokenHandler typedef from GACAppCheck.h.
@andrewheard andrewheard merged commit 4d67aa1 into app-check-core Jul 24, 2023
53 checks passed
@andrewheard andrewheard deleted the ah/app-check-core-interop branch July 24, 2023 21:49
@firebase firebase locked and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants