Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Cordova 9 support #134

Merged
merged 1 commit into from
May 27, 2019
Merged

Cordova 9 support #134

merged 1 commit into from
May 27, 2019

Conversation

pcsantana
Copy link
Contributor

What

Cordova 9 support! Fixes #132

Verification Steps

  1. npm install cordova@latest
  2. ionic cordova platform rm android
  3. Add this plugin (from forked repo to test)
  4. ionic cordova platform add android@latest

The build succeed!

Checklist:

  • Code has been tested locally by PR requester
  • Changes have been successfully verified by another team member

Progress

  • Finished task

@pcsantana pcsantana mentioned this pull request May 22, 2019
3 tasks

var platformRoot = path.join(ctx.opts.projectRoot, 'www');
var settingsFile = path.join(platformRoot, 'google-services.json');

var platformAndroid = 'platforms/android';

if (fs.existsSync('platforms/android/app')) {
if (fs.existsSync('platforms/android/app/src/main')) {
Copy link
Collaborator

@danielpassos danielpassos May 23, 2019

Choose a reason for hiding this comment

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

I'm confused about this change. If the platforms/android/app/src/main exists we should use 'platforms/android/app' or it should be update also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had some experiences where some plugins creates an "app" folder. In that case, I would not be able to identify whether cordova-android >= 7.x or not.
The condition is only to try to ensure this. Maybe have another way to do it.

@danielpassos danielpassos self-requested a review May 27, 2019 20:43
Copy link
Collaborator

@danielpassos danielpassos left a comment

Choose a reason for hiding this comment

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

I have tested if using 9.0.0 (cordova-lib@9.0.1) and it works like a charm. Thanks for the contribution @pcsantana I really appreciate that!

@danielpassos danielpassos merged commit a77ef61 into aerogear:master May 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update to the latest version of Cordova
2 participants