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

Experiment - Android X migration #1112

Merged
merged 71 commits into from
Jul 10, 2019
Merged

Experiment - Android X migration #1112

merged 71 commits into from
Jul 10, 2019

Conversation

marecar3
Copy link
Contributor

@marecar3 marecar3 commented Jun 10, 2019

Issue: #1062

With this PR we are migrating gutenberg-mobile android application to AndroidX.

RN already provided AndroidX support so we are targeting RN60 with @jtreanor's patch for cocoapods in this PR

Libraries that are also migrated to AndroidX:

  1. Aztec-Android

  2. react-native-video

  3. react-native-recyclerview-list

WPAndroid against this PR.

Note: As we are using the newest version of jsc-android 241213, should we push it on : https://dl.bintray.com/wordpress-mobile/react-native-mirror/org/webkit/android-jsc/ ? @hypest @jtreanor (not sure)

Not working: Working ✅

  1. Gutenberg mobile tests, enzyme complains that adapter isn't set, I assume that it's something with RN60 version, maybe @etoledom can help here with some hint?

  2. WordPress-Android tests -> jitpack.io can't build react-native-video-wordpress as it can't find RN60 at https://dl.bintray.com/wordpress-mobile/react-native-mirror/com/facebook/react/react-native/

@hypest I am curios should we push RN60 there or maybe I just wrongly configured react-native-video wordpress-mobile/react-native-video@enable-android-build-of-standalone-package...wordpress-mobile:experiment/android_x_migration#diff-3a56ae3cf3e05886fec69653e96f2456R53

I must admit that I am not sure how build.gradle file should be configured as I saw that jitpack is trying to call this command: ./gradlew clean -Pgroup=com.github.wordpress-mobile -Pversion=57ad4e840a -xtest -xlint install in react-native-video.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@marecar3 marecar3 added this to the v1.7 milestone Jun 10, 2019
@marecar3 marecar3 changed the title Issue/android x migration Experiment - Android X migration Jun 10, 2019
@hypest
Copy link
Contributor

hypest commented Jun 11, 2019

Update: I pushed RN v0.60.0-rc.1, JSC-r241213 to the Bintray mirror.

@hypest
Copy link
Contributor

hypest commented Jun 11, 2019

WPAndroid PR, at least for testing purposes: wordpress-mobile/WordPress-Android#10022

@hypest
Copy link
Contributor

hypest commented Jun 11, 2019

I tried to build the latest gutenberg-mobile develop with the wpandroid develop using binary artifacts (source mode off) and it works (including running the app and the editor) so, this PR is not a blocker for the v1.7 code freeze. Besides, this PR updates React Native to a RC version which we don't want to ship yet so, better to postpone anyway.

@hypest hypest modified the milestones: v1.7, v1.8 Jun 11, 2019
@marecar3 marecar3 added [OS] iOS and removed [Status] DO NOT MERGE Do not merge this PR labels Jul 8, 2019
@etoledom
Copy link
Contributor

etoledom commented Jul 8, 2019

iOS side looks good to go! ✅ 🎉

Tested on the demo app and WPiOS via wordpress-mobile/WordPress-iOS#12056

@hypest
Copy link
Contributor

hypest commented Jul 9, 2019

As far as I can see, the Aztec PR is not green yet and needs more migration of the testsuites. I pushed a commit to fix some of them but more work is needed. Let's get it green before we merge the rest of the PR set so we can be more confident we haven't broken something.

@marecar3
Copy link
Contributor Author

marecar3 commented Jul 9, 2019

As far as I can see, the Aztec PR is not green yet and needs more migration of the testsuites. I pushed a commit to fix some of them but more work is needed. Let's get it green before we merge the rest of the PR set so we can be more confident we haven't broken something.

Ok, I will take care of that one @hypest

So we don't need to make the client app directly import React Native
classes.
@hypest
Copy link
Contributor

hypest commented Jul 10, 2019

👋 @daniloercoli , can you do another round of testing to mainly verify I didn't break something with 4107bf6? Thanks!

Copy link
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

This now looks good to me!

@marecar3 marecar3 merged commit ec73e52 into develop Jul 10, 2019
@marecar3 marecar3 deleted the issue/android_x_migration branch July 10, 2019 18:30
@marecar3 marecar3 mentioned this pull request Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants