-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Update androidx #1626
Update androidx #1626
Conversation
examples/basic/android/app/src/main/java/com/videoplayer/MainApplication.java
Outdated
Show resolved
Hide resolved
Oh, it is my mistake, I used react-native link twice |
I will make a commit tomorrow |
Thanks for this @vokhuyetOz. Looks like this forces people to use AndroidX? Or is this backwards compatible? (I'm not too familiar with Android :p ) |
Now, people have to use AndroidX whenever upgrade to new build tool version. As I know, google moves android support responsibilities to new location |
From yesterday, I couldn’t build android app, the error is can’t import v4 AppCompat |
Have you upgraded to RN 0.60RC? From my understanding this will only be required after upgrading to RN 0.60 react-native-community/discussions-and-proposals#129 |
And btw, I installed from your branch and then the android build fails. So it's not backwards compatible. |
No, i still use 0.59 |
I just run it in example, i will check it again tomorrow, thank you for your report :) |
@jenshandersson which error did you have? |
make sure that you have updated your gradle.properties and app/build.gradle. |
Waiting for this PR :) Thank you @vokhuyetOz |
@vokhuyetOz after doing your changes I get compile errors in other 3rd party libs... I assume those haven't been upgraded to use AndroidX then? One of them is react-native-gesture-handler (that is the error I'm getting as well).
How do you propose I/we move forward with this? If we would merge this PR then a lot of people would have the same issues, right? (Sorry again for being a bit slow on the Android side) |
a lot of libraries must migrate to AndroidX to fix it |
i am also making a pull request for another library |
i think i should make a PR for react-native-gesture-handler to fix it |
@WrathChaos could you test this project? |
@vokhuyetOz I already tried it, other libraries also crushes :( We need to update all of them... AndroidX ruined my day.. |
you should create a new project and check only this library :) |
@vokhuyetOz I will try it when I have time :) Thank you for this contribution anyway. |
@vokhuyetOz after speaking to @kelset I now understand that AndroidX upgrade is not required. Since rn-video has a Can you please create a new PR with setting explicit versions instead. Upgrading to 28 is fine. But we can't force a AndroidX upgrade of all users right now. See this PR: react-native-device-info/react-native-device-info@9588763 We will keep this PR as it will be required to move to AndroidX for RN 0.60, but let's wait with that. |
Thank you, at the moment, it is not required, but from 11/2019 developers must update their apps whenever create or update new version. I will take a look at react native device info and make a change |
@jenshandersson please check out #1629 . thank you ! |
react-native 0.60 has now its first stable release. Should merge this asap and publish a new major release. |
This PR will make also possible to update exoplayer to 2.10.x? or totally unrelated? |
Can we align with this PR also? |
Yes, of course |
@vokhuyetOz does this change the min version of React required? |
@CHaNGeTe no, it doesn't relate to min version of react required, sorry for my late. |
hey @ashnfb @cobarx @n1ru4l @jenshandersson I need another approval for this one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legit for breaking change release (major version number bump)
What makes it breaking change? (if RN version required does not change) |
Doesn’t this force the user into changing their grade setup/ using jetifier? |
ok! clear now! |
Make installation parts easier to link
fix typo
Make more obvious the changes needed via using diff
fix indent
@vokhuyetOz I am preparing the merge. Do you know the minimum required gradle version for this to work? |
Aha it's good to know this PR will merge |
Woohoo! 🎉 |
android-exoplayer/build.gradle
Outdated
implementation "com.android.support:support-annotations:${safeExtGet('supportLibVersion', '+')}" | ||
implementation "com.android.support:support-compat:${safeExtGet('supportLibVersion', '+')}" | ||
implementation "com.android.support:support-media-compat:${safeExtGet('supportLibVersion', '+')}" | ||
// implementation "com.android.support:support-annotations:${safeExtGet('supportLibVersion', '+')}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be removed instead of commented out?
@@ -108,6 +111,7 @@ android { | |||
release { | |||
signingConfig signingConfigs.debug | |||
minifyEnabled enableProguardInReleaseBuilds | |||
matchingFallbacks = ['release', 'debug'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't remember exactly why i add it. i will remove it in the next commit :D
sorry, i don't find any information about the minimum required gradle version. But i find Hope it helps you! |
Just migrating to AndroidX