Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Add dependency on cordova-support-google-services #2293

Merged
merged 1 commit into from Apr 9, 2018
Merged

Add dependency on cordova-support-google-services #2293

merged 1 commit into from Apr 9, 2018

Conversation

chemerisuk
Copy link
Contributor

Adding dependency on cordova-support-google-services should resolve some conflicts.

Description

Per our discussion at chemerisuk/cordova-support-google-services#1 (comment).

Related Issue

chemerisuk/cordova-support-google-services#1

Motivation and Context

Solve compatibility issues between cordova plugins.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@chemerisuk
Copy link
Contributor Author

@macdonst let me know if I can help to update documentation.

@macdonst macdonst merged commit fe6cb91 into phonegap:master Apr 9, 2018
@macdonst
Copy link
Member

macdonst commented Apr 9, 2018

@chemerisuk thanks for the PR. I just merged it in. We should try and get as many people as possible to move to using your plugin as a dependency. Actually, we should probably make it an option you can enabled in cordova-android.

@chemerisuk
Copy link
Contributor Author

chemerisuk commented Apr 9, 2018

@macdonst agree, It's hard to imagine modern Android development without having a dependency on google services. The advantage of gradle plugins is that they do not add extra code at runtime, so people that for a some reason do not have google-services.json won't notice any difference.

The only issue is compatibility with many existing plugins... On the other hand recent cordova-android 7 breaks plugins anyway, so may be it's a good time to to integrate cordova-support-google-services into the core. Thoughts?

@fredgalvao
Copy link
Collaborator

@chemerisuk You should try and spread that word on the cordova dev mailing list. We'll get broader opinions there.

@macdonst
Copy link
Member

I agree with @fredgalvao, we should bring this to the cordova dev mailing list. What I envision is a preference like:

<preference name="android-RunGoogleServicesPlugin" value="true"/>

where the default it is false. Then any app that needs the plugin to run will enable this preference.

Say this lands in cordova-android 8 then @chemerisuk can update the cordova-support-google-services so it only installs in an app when cordova-android is less than 8. That way plugin devs who use the cordova-support-google-services won't even need to update their plugin.xml files.

Also, @chemerisuk what do you mean by cordova-android 7 breaks plugins?

@chemerisuk
Copy link
Contributor Author

@macdonst folder structure has changed and in many cases plugins must be updated because of that (read http://cordova.apache.org/announcements/2017/12/04/cordova-android-7.0.0.html#disqus_thread).

As for preference android-RunGoogleServicesPlugin - it totally makes sense. I was thinking first about always adding the gradle plugin (because it doesn't add any runtime code), but after testing on empty project it seems like the gradle plugin fails without google-services.json:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:processDebugGoogleServices'.
> File google-services.json is missing. The Google Services Plugin cannot function without it. 
   Searched Location: 
  /.../git/test/platforms/android/app/src/nullnull/debug/google-services.json
  /.../git/test/platforms/android/app/src/debug/nullnull/google-services.json
  /.../git/test/platforms/android/app/src/nullnull/google-services.json
  /.../git/test/platforms/android/app/src/debug/google-services.json
  /.../git/test/platforms/android/app/src/nullnullDebug/google-services.json
  /.../git/test/platforms/android/app/google-services.json

@macdonst
Copy link
Member

@chemerisuk I just sent a message out to the dev mailing list. Please chime in.

@chemerisuk
Copy link
Contributor Author

chemerisuk commented Apr 21, 2018

@macdonst I have issue with connecting to that discussion... The only suggestion is to drop android- prefix from the preference name - it seems like new cordova preferences use only camel case style, may be better to be consistent. Something like RunGoogleServicesPlugin or EnableGoogleServicesPlugin.

@macdonst
Copy link
Member

@chemerisuk sorry are you saying you can't participate in the discussion? With regards to the exact name of the preference I'm not worried about what it eventually gets called.

@chemerisuk
Copy link
Contributor Author

@macdonst had problems with authentication, now resolved.

@chemerisuk
Copy link
Contributor Author

@macdonst Just submitted PR at apache/cordova-android#438

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants