Skip to content

Fix plugin remove issue#923

Open
Jule- wants to merge 6 commits into
arnesson:masterfrom
Jule-:master
Open

Fix plugin remove issue#923
Jule- wants to merge 6 commits into
arnesson:masterfrom
Jule-:master

Conversation

@Jule-

@Jule- Jule- commented Oct 24, 2018

Copy link
Copy Markdown

Like discussed with @briantq in #898, this PR should be harmless but need other people to test it in their projects.

@Jule- Jule- changed the title Fix plugin remove issue #898 Fix plugin remove issue Oct 24, 2018
@Jule-

Jule- commented Oct 24, 2018

Copy link
Copy Markdown
Author

Ok maybe we can't move <framework /> dependencies for old cordova version...

EDIT: to be confirmed

@Jule-

Jule- commented Oct 24, 2018

Copy link
Copy Markdown
Author

@briantq I think I know why there is a dummy google-services.json... For test build to succeed I think...

By the way it seems like tests are somehow broken because build fail for cordova 8.0.0 and it is detected as success:

BUILD FAILED in 24s
(node:4538) UnhandledPromiseRejectionWarning: Error: /home/travis/build/arnesson/cordova-plugin-firebase/.build-android/platforms/android/gradlew: Command failed with exit code 1 Error output:
FAILURE: Build failed with an exception.
* What went wrong:
A problem occurred configuring root project 'android'.
> Could not resolve all files for configuration ':classpath'.
   > Could not find intellij-core.jar (com.android.tools.external.com-intellij:intellij-core:26.0.1).
     Searched in the following locations:
         https://jcenter.bintray.com/com/android/tools/external/com-intellij/intellij-core/26.0.1/intellij-core-26.0.1.jar
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
* Get more help at https://help.gradle.org
BUILD FAILED in 24s
    at ChildProcess.whenDone (/home/travis/build/arnesson/cordova-plugin-firebase/.build-android/platforms/android/cordova/node_modules/cordova-common/src/superspawn.js:169:23)
    at emitTwo (events.js:126:13)
    at ChildProcess.emit (events.js:214:7)
    at maybeClose (internal/child_process.js:915:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)
(node:4538) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:4538) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
The command "bash ./test/test-default.sh $CORDOVA_VERSION $CORDOVA_PLATFORM $CORDOVA_PLATFORM_VERSION" exited with 0.

Plus I don't have issues building on my environment with com.android.tools.external.com-intellij:intellij-core:26.0.1.

@Jule-

Jule- commented Oct 24, 2018

Copy link
Copy Markdown
Author

Ok issue in cordova-android@7.1.0 fixed in cordova-android@7.1.1, see https://issues.apache.org/jira/browse/CB-14127.

@Jule-

Jule- commented Oct 24, 2018

Copy link
Copy Markdown
Author

Ok I try to patch tests the right day...
https://forum.ionicframework.com/t/ionic-pro-package-failed-after-successful-deploy/145355/35
https://ionic.zendesk.com/hc/en-us/articles/360005529314

So it is working just need to wait new release cordova-android@7.1.2...

@Jule-

Jule- commented Oct 24, 2018

Copy link
Copy Markdown
Author

Just saying but cordova-paramedic project looks promising in order to externalize CI boilerplate stuff and write automated tests!
Overview: https://kerrishotts.github.io/pgday/workshops/2017/campp/testing.html#cordova-paramedic

@briantq

briantq commented Oct 26, 2018

Copy link
Copy Markdown
Contributor

@soumak77 might want to weigh in on the CI suggestion. I have not touched that area yet. I have been pretty slammed with my day job but hope to catch up on code reviews this weekend.

@Jule- Jule- force-pushed the master branch 3 times, most recently from 181e8a1 to b04abb8 Compare November 8, 2018 20:10
@Jule-

Jule- commented Nov 8, 2018

Copy link
Copy Markdown
Author

@briantq tests are working now! Thanks to cordova-android@7.1.2 release as expected. Could you please merge this PR now?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants