You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/08/18 12:00:12 UTC

[GitHub] [cordova-android] HansKrywaa opened a new pull request #1052: breaking: only support androidx

HansKrywaa opened a new pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052


   <!--
   Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
   
   http://cordova.apache.org/contribute/contribute_guidelines.html
   
   Thanks!
   -->
   
   ### Platforms affected
   
   Android
   
   ### Motivation and Context
   <!-- Why is this change required? What problem does it solve? -->
   <!-- If it fixes an open issue, please link to the issue here. -->
   
   As discussed in #841 , @breautek told me that Version **10.0.0** is planned to only support AndroidX. With this it is possible to use Android Fragments.
   
   ### Description
   <!-- Describe your changes in detail -->
   
   - `android.useAndroidX` and `android.enableJetifier` are now set to **true** by default.
   - The `AndroidManifest.xml`'s Activity Theme changed to the AppCompat one
   - `androidx.appcompat:appcompat:1.2.0` added as depencency
   - Usages of `android.app.Activity` in all Java Files replaced by `androidx.appcompat.app.AppCompatActivity`
   - Remove testing of normal `android`-Project
   
   ### Testing
   <!-- Please describe in detail how you tested your changes. -->
   
   `npm test` and manual test with an example project of a plugin of mine, which works with `AppCompatActivity`. The Preference `androidXEnabled` is not used anymore.
   
   ### Checklist
   
   - [x] I've run the tests to see all new and existing tests pass
   - [ ] I added automated test coverage as appropriate for this change
   - [ ] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
   - [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
   - [ ] I've updated the documentation if necessary
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675918524


   @faugusztin I don' think it changes the `cordova-android` Base Classes to use **AppCompatActivity** instead of `android.app.Activity`.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa edited a comment on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa edited a comment on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675651004


   @dpogue we discussed about supporting both at the same time before (as you can see in the linked error) - i mailed the Dev Mailing List about this, but didn't get an answer. Maybe better Solution to make sure Plugins not automatically break?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675930509


   Have a Look into this PR. I changed the **CordovaActivity** to extend **AppCompatActivity** and not normal **Activity** anymore. I think this is the Change that will break many Plugins and won't be fixed by the androidx-adapter-plugin 🤔 or am i wrong


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] breautek commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-676300697


   > Have a Look into this PR. I changed the **CordovaActivity** to extend **AppCompatActivity** and not normal **Activity** anymore. I think this is the Change that will break many Plugins and won't be fixed by the androidx-adapter-plugin  or am i wrong
   
   Yah once the framework depends on the androidx support library, this forces everyone, and any plugins they use to also use androidx support libraries rather than the old deprecated support libraries. This is essentially anything inside the `android.support` namespace.
   
   Therefore, in order to support this in an non-breaking way, we would need to start templating/have two versions of the framework, so that we have a androidx version of the codebase and a non-androidx version of the codebase.
   
   The good news is, if the library is using version 28 of the support libs... then they can easily be upgraded to androidx 1.0.0, as they are [binary equivalent](https://developer.android.com/jetpack/androidx/migrate#prerequisites). However, like dpogue says, they are a lot of unmaintained plugins that will end up breaking.
   
   To me, while this does pose a problem, but I think holding back poses a larger problem. I don't think holding Cordova back because of unmaintained plugins is a good idea. One of the goals of androidx is to improve android reliability by reducing the need to use reflections in the android code, while still being able to support newer features/APIs and remain backwards compatibility just like before. Moving forward, Google is going to be [more strict](https://android-developers.googleblog.com/2018/02/improving-stability-by-reducing-usage.html) on reflection usage I think. Holding back will put us in a situation where it may become impossible to support newer features as newer API levels are released.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] EinfachHans commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
EinfachHans commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-819368298


   @erisu @knight9999 added 😊 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] faugusztin commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
faugusztin commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675912608


   @HansKrywaa isn't that what the plugin https://github.com/dpa99c/cordova-plugin-androidx-adapter/ does ? To replace old Support Library imports with new AndroidX imports.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-678619558


   @breautek Yeah think so too. Maintaining both will be to much work , (just) for unmaintained Plugins


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675651004


   @dpogue we discussed about supporting both at the same time before (as you can see in the linked error) - i mailed the Dev Mailing List about this, but dodn't get an answer. Maybe better Solution to make sure Plugins not automatically break?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] erisu commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
erisu commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-819377815


   @EinfachHans Thank you for the fix.
   
   Can you also rebase the branch and resolve the conflicted files?
   
   I believe three of the files you deleted so it should be easy to fix, but the Java file I have not looked at so it might be easy to fix too.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] faugusztin commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
faugusztin commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675923810


   @HansKrywaa dpogue's comment was about "unmaintained community plugins". Plugin source codes go into platforms/android/app/src/main, so they are updated to AndroidX imports using the androidx-adapter plugin.
   
   So what could remain as a possible issue are gradle files introduced by plugins, but by the nature of old Android Support Library being abandoned by Google anyway, they will have to be left behind; or people will have to make their own patched versions fixing the Gradle issues, if there are any.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] dpogue commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
dpogue commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675507807


   @breautek I think this idea needs further discussion on the dev mailing list. My concern is that using AndroidX by default will break a **lot** of community plugins, because anything that uses the support library will be incompatible and cause build failures.
   
   Because plugin source files are copied into the app project instead of being distributed as jar/aar files, tools like Jetifier won't upgrade the references to the support libraries.
   
   I'm not saying we shouldn't do this (we probably should), but it needs a discussion about whether we're willing to break a lot of unmaintained plugins for a lot of people.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] HansKrywaa commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
HansKrywaa commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675880176


   @dpogue - Yeah i know, what i mean was, that it (probably in the prepare script) can replace the Usages of `android.app.Activity` to `androidx.appcompat.app.AppCompatActivity` and adjust the `AndroidManifest.xml` if `androidXEnabled` is set. With a logic like this (or another with the same result) we can use androidx Library and AppCompatActivity only when `androidXEnabled` is **true** , so this should break anything?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] dpogue commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
dpogue commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675854520


   > we discussed about supporting both at the same time before
   
   It's not possible for a project to use both AndroidX and the Android support libraries at the same time.
   
   Currently cordova-android support picking one or the other via a preference in config.xml. *However*, it is not possible to write a plugin that supports both. You either need two versions of every plugin that uses the libs to npm, or you have plugins that only work with one configuration option.
   
   There are a lot of apps using 3rd party plugins with dependencies on the Android support libraries, and several of those plugins aren't actively maintained. There is no way to use those plugins in an app that has opted in to AndroidX.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] dpogue edited a comment on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
dpogue edited a comment on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-675854520


   > we discussed about supporting both at the same time before
   
   It's not possible for a project to use both AndroidX and the Android support libraries at the same time.
   
   Currently cordova-android support picking one or the other via a preference in config.xml. *However*, it is not possible to write a plugin that supports both. You either need two versions of every plugin that uses the libs published to npm, or you have plugins that only work with one configuration option.
   
   There are a lot of apps using 3rd party plugins with dependencies on the Android support libraries, and several of those plugins aren't actively maintained. There is no way to use those plugins in an app that has opted in to AndroidX.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] erisu merged pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
erisu merged pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


[GitHub] [cordova-android] brodybits commented on pull request #1052: breaking: only support androidx

Posted by GitBox <gi...@apache.org>.
brodybits commented on pull request #1052:
URL: https://github.com/apache/cordova-android/pull/1052#issuecomment-813826656


   > I don't think holding Cordova back because of unmaintained plugins is a good idea.
   
   I think another benefit of moving forward could be to encourage user community members to fork and publish modernized, supported versions of some unmaintained plugins. Beauty of open source!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org