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

firestore_exceptions.h: remove dependency on absl to set FIRESTORE_HAVE_EXCEPTIONS #14442

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

dconeybe
Copy link
Contributor

firebase-cpp-sdk has some issues compiling absl-dependent code on Android; therefore, remove the dependency on absl from firestore_exceptions.h, as it was before #14348.

See https://github.com/firebase/firebase-ios-sdk/pull/14348/files/1989942cffe222e986230c1539908a9fd105360c#r1950085884

#no-changelog

… from commit 64d50a7, but with a new-and-improved comment.
@dconeybe
Copy link
Contributor Author

firebase-cpp-sdk test run to demonstrate that this PR fixes the android build breakage: https://github.com/firebase/firebase-cpp-sdk/actions/runs/13254450771/job/36998710412?pr=1689

Copy link
Contributor

@jonsimantov jonsimantov left a comment

Choose a reason for hiding this comment

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

Looks good. Can you keep this branch open until the C++ release is out in the next day or two? I temporarily changed the Firestore external reference to point directly to it and I don't want it to go away before the package is finished building.

@dconeybe
Copy link
Contributor Author

Looks good. Can you keep this branch open until the C++ release is out in the next day or two? I temporarily changed the Firestore external reference to point directly to it and I don't want it to go away before the package is finished building.

I think the dconeybe/AndroidAbslExceptionsFix branch will be automatically deleted when this PR is merged. I can definitely delay the merge until you are done with it, since the cpp sdk is the only code (that I am aware of) that cares about this PR. Or you can point your code at the commit hash in the main branch after this PR is merged. Your call.

…nimum-supported ndk version) instead of r16b
@jonsimantov
Copy link
Contributor

Hmm, let's use the commit hash then. I'll make the change on the C++ side.

@dconeybe dconeybe merged commit 25e724d into main Feb 11, 2025
35 checks passed
@dconeybe dconeybe deleted the dconeybe/AndroidAbslExceptionsFix branch February 11, 2025 20:17
@dconeybe
Copy link
Contributor Author

Hmm, let's use the commit hash then. I'll make the change on the C++ side.

Ok. PR has been merged. The commit hash is 25e724d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants