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

Fix plugin remove issue #923

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix plugin remove issue #923

wants to merge 6 commits into from

Conversation

Jule-
Copy link

@Jule- Jule- commented Oct 24, 2018

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-
Copy link
Author

Jule- commented Oct 24, 2018

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

EDIT: to be confirmed

@Jule-
Copy link
Author

Jule- commented Oct 24, 2018

@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-
Copy link
Author

Jule- commented Oct 24, 2018

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-
Copy link
Author

Jule- commented Oct 24, 2018

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-
Copy link
Author

Jule- commented Oct 24, 2018

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
Copy link
Contributor

briantq commented Oct 26, 2018

@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-
Copy link
Author

Jule- commented Nov 8, 2018

@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