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

Allow Specifying a different version for base and maps on android #2587

Merged
merged 11 commits into from Jan 8, 2019

Conversation

Projects
None yet
4 participants
@Titozzz
Copy link
Contributor

commented Nov 15, 2018

Does any other open PR do the same thing?

No

What issue is this PR fixing?

#2584

How did you test this PR?

This allows android to specify a custom version for
com.google.android.gms : play-services-maps
and
com.google.android.gms : play-services-base
by specifying

	googlePlayServicesVersion = "16.0.1"
    googlePlayServicesAuthVersion = "16.0.1"// this is how react-native-google-signin does 
    googlePlayServicesMapsVersion = "16.0.0"

for example I had to use 16.0.1 for base but maps does not exist for that version
https://mvnrepository.com/artifact/com.google.android.gms/play-services-maps

It should automatically fallback to googlePlayServicesVersion value if googlePlayServicesMapsVersion is not specified

Titozzz added some commits Jan 10, 2018

Fix Google map podspec
For use in cocoapods only projects
Merge pull request #2 from react-community/master
Update react-native maps
def googlePlayServicesVersion = rootProject.hasProperty('googlePlayServicesVersion') ? rootProject.googlePlayServicesVersion : DEFAULT_GOOGLE_PLAY_SERVICES_VERSION
def androidMapsUtilsVersion = rootProject.hasProperty('androidMapsUtilsVersion') ? rootProject.androidMapsUtilsVersion : DEFAULT_ANDROID_MAPS_UTILS_VERSION
def googlePlayServicesVersion = rootProject.hasProperty('googlePlayServicesVersion') ? rootProject.googlePlayServicesVersion : DEFAULT_GOOGLE_PLAY_SERVICES_VERSION
// googlePlayServicesMapsVersion > googlePlayServicesVersion > DEFAULT_GOOGLE_PLAY_SERVICES_MAPS_VERSION (16.0.0)

This comment has been minimized.

Copy link
@rborn

rborn Nov 20, 2018

Collaborator

do we need this commented code?

This comment has been minimized.

Copy link
@Titozzz

Titozzz Nov 21, 2018

Author Contributor

It's not code, it was a comment so that people understand the line below. It can remove it completely

This comment has been minimized.

Copy link
@rborn

rborn Nov 21, 2018

Collaborator

Maybe you could make then more clear (more descriptive) or delete it, your call 😄

@rborn

This comment has been minimized.

Copy link
Collaborator

commented Nov 20, 2018

@Titozzz could you check with @yeomann maybe use safeExtGet like in #2398

Thnx

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2018

Well I could import his pr inside of mine if you want me to, or wait for his to be merged

@rborn

This comment has been minimized.

Copy link
Collaborator

commented Nov 21, 2018

@Titozzz let's wait for @yeomann to reply, and if not maybe you can import the PR in yours and push again. 🤗

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 22, 2018

Sure thing 😄

@yeomann

This comment has been minimized.

Copy link

commented Nov 26, 2018

hey guys, sorry for delay. sure thing mate @Titozzz go ahead 😃

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2018

Will do :)

Titozzz added some commits Nov 30, 2018

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Nov 30, 2018

@rborn Done

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

Still nothing ? 😢

@rborn rborn merged commit 2e785e9 into react-native-community:master Jan 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rborn

This comment has been minimized.

Copy link
Collaborator

commented Jan 8, 2019

@Titozzz sorry man, vacations + very busy, done now ❤️

@Titozzz

This comment has been minimized.

Copy link
Contributor Author

commented Jan 8, 2019

Don't worry ❤️ This was part of emptying my github gigantic pile of notifications after vacations :D !

Thanks ! you can close #2584

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.