[face-detector] Make Firebase pods version customisable, so it can be used with bare projects #7141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mczernek
reviewed
Feb 21, 2020
IjzerenHein
force-pushed
the
@hein/expo-firebase/facedetector-version
branch
2 times, most recently
from
1144653
to
8656166
Compare
sjchmiela
approved these changes
May 13, 2020
|
Native Component List for this branch is ready |
This makes the firebase pod version overridable so that this package can be used in bare projects, together with other firebase packages. When Firebase versions differ accross multiple packages, pod install will fail. This follows the `$FirebaseSDKVersion` constants also used by react-native-firebase to maximize interop and make it easier for users to configure. [face-detector] Remove explicit FirebaseMLVision dependencies [face-detector] Update changelog Update packages/expo-face-detector/CHANGELOG.md Co-authored-by: Stanisław Chmiela <sjchmiela@users.noreply.github.com>
IjzerenHein
force-pushed
the
@hein/expo-firebase/facedetector-version
branch
from
9f3034b
to
61f169a
Compare
@sjchmiela Rebased on master and re-installed pods 👍 |
sjchmiela
approved these changes
May 14, 2020
Co-authored-by: Stanisław Chmiela <sjchmiela@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
This PR makes the Firebase Pods version for
expo-face-detectorcustomisable through aFirebaseSDKVersionconstant. This allows for using this package in bare projects side by side with other packages such asreact-native-firebaseorexpo-firebase-analytics, that possibly specify a different version in their Pod file.How
This adds the optional use of a constant called
FirebaseSDKVersionto your podfile. When existent, it uses the firebase version as defined in the constant. Otherwise it uses the default version. This constant is the same as defined inreact-native-firebasemaking interop & configuration easier.This PR also removes the explicit pod references to
FirebaseMLVisionandFirebaseMLCommon, which are imported implicitly byFirebase/MLVisionand were added in #5685. Explicitly defining these kinds of implicit dependencies seems like a bad strategy and gives only partial guarantees. If you would want a full guarantee, then you would need to explicitly define every implicitly imported Pod dependency, which is not a feasible thing to do (imo).Test Plan
expo-face-detectorand@react-native-firebase/appto a bare projectcd ios && pod install)$FirebaseSDKVersion = 'x.x.x'at the top of the Podfilecd ios && pod install)