Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) by ParaskP7 · Pull Request #12 · wordpress-mobile/react-native-gesture-handler

ParaskP7 · 2022-02-16T10:35:40Z

This PR is part of Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) PR. There are 13 libraries that are upgraded to Gradle 7.4 & AGP 7.1.1 (& Disable Jetifier).

All those PRs follow the general outline below:

Gradle version upgraded to 7.3.3 with the ./gradlew wrapper --gradle-version=7.3.3 --distribution-type=all command.
AGP version upgrade to 7.0.4 (see android/settings.gradle.kts change).
Build output diff:
- To identify and fix new warnings/errors that got added.
- To identify and verify old warnings/errors that got removed.
Lint output diff
- To identify and fix new warnings/errors that got added.
- To identify and verify old warnings/errors that got removed.
The above 4 steps were redone with a follow-up update to Gradle 7.4 & AGP 7.1.1.
Jetifier drop (see android/gradle.properties change).
OUTDATED ❌
- Version bump to 1.10.1-wp-5 (see package.json).
- Tarball update to 1.10.1-wp-5 (see react-native-gesture-handler-1.10.1-wp-5.tgz).
- The build was also verified through Jitpack (see build.log).
OUTDATED ❌
- AGP version upgrade to 7.1.1 (from 4.1.0) (within android/build.gradle). Cc @oguzkocer
- Version bump to 2.2.0-wp-4 (see package.json).
- Tarball update to 2.2.0-wp-4 (see react-native-gesture-handler-2.2.0-wp-4.tgz).
- The build was also verified through Jitpack (see build.log).
UPDATED ✅
- Use common interface from reanimated (for more deals see here).
- Version bump to 2.2.0-wp-5 (see package.json).
- Tarball update to 2.2.0-wp-5 (see react-native-gesture-handler-2.2.0-wp-5.tgz).
- The build was also verified through Jitpack (see build.log).

To test - Now

These changes can be tested as part of the gutenberg PR which is updated to use the temporarily non-tagged node module project dependencies generated with these changes.

To test - Later ⚙️

These changes can be tested as part of the gutenberg-mobile PR ⚙️ or WordPress-Android PR ⚙️ which is updated to use the bundle ⚙️ generated with these changes.

FYI: It's best to leave the testing step to the gutenberg PR review since even if these PRs are merged, they won't impact anything until the package.json file is updated. Worst case scenario, a follow up PR will get opened to fix any issues that are found during testing.

Follow up

Once these PRs are merged in, a new tag will get created for each library and the package.json file will be again updated accordingly.

The 'com.dipien.byebyejetifier' plugin was used to determine whether Jetifier can be disable for this project. After configuring 'Bye Bye Jetifier' and running the below command: ./gradlew canISayByeByeJetifier -Pandroid.enableJetifier=false The output was clear, Jetifier can be now safely disabled for this project: > Task :canISayByeByeJetifier ========================================= Project: : ========================================= * No legacy android support usages found ========================================= Project: :lib ========================================= * No legacy android support usages found ===================================================== ... * No dependencies with legacy android support usages! ... ===================================================== ... ... =============================== ... You can say Bye Bye Jetifier. * ... ===============================

# Conflicts: # react-native-gesture-handler-1.10.1-wp-5.tgz

…sture-handler into update/gradle-to-7.3.3-agp-to-7.0.4 � Conflicts: � android/gradle.properties � package.json

ParaskP7 · 2022-03-03T14:55:36Z

👋 @oguzkocer

AGP version upgrade to 7.1.1 (from 4.1.0) (within android/build.gradle). Cc @oguzkocer

You will notice that I cc'ed you on the description because I want you to be aware of this change since it might revert the change you did by creating settings.gradle.kts and using pluginManagement instead of doing that directly through buildscript. Wdyt?

PS: This change was done because the wp-fork was recently updated from 1.10.1-wp-4 to 2.2.0-wp-3, a major upgrade on this branch, lots of diffs, which I then pulled in this PR to keep it up-to-date. Cc @fluiddot

oguzkocer · 2022-03-07T16:37:14Z

@ParaskP7 I've added plugin DSL support to this project as well as many other react-native fork projects because I wanted react-native-bridge the ability to set the plugin version for easy testing. In my humble opinion reverting the change is counter-productive and 71885ee by @fluiddot was a mistake.

@fluiddot The merge conflict was a very easy one to address, so I was wondering why you have opted to make this change. Is it at all possible you weren't familiar with the plugin DSL? If so, we can fix that easily in this PR. However, if it was intentional, I hope we can discuss it before we commit to the version in this PR.

fluiddot · 2022-03-07T16:47:25Z

@fluiddot The merge conflict was a very easy one to address, so I was wondering why you have opted to make this change. Is it at all possible you weren't familiar with the plugin DSL? If so, we can fix that easily in this PR. However, if it was intentional, I hope we can discuss it before we commit to the version in this PR.

Just to clarify, do you refer to the change in the plugins block?

Before:

plugins {
    id "com.android.library"
    id "kotlin-android"
    id "maven-publish"
}

After:

apply plugin: 'com.android.library'
apply plugin: 'kotlin-android'
apply plugin: 'maven-publish'

If so, it was probably a mistake from my side. I'm still getting familiar with Gradle DSL, so please let me know if there's anything else incorrect in the configurations 🙇 .

oguzkocer · 2022-03-07T16:56:20Z

@fluiddot That's part of it, but in short, we shouldn't have a buildscript block which is the old notation. For example, classpath("com.android.tools.build:gradle:4.1.0") is not necessary, because the version is set in the settings.gradle.kts like so. Kotlin plugin declaration should also be done in the settings.gradle.kts file similar to this.

Hope that clarifies it and if you'd like to learn more about the plugin DSL, you might find this documentation useful.

ParaskP7 · 2022-03-09T12:12:11Z

👋 @oguzkocer !

@fluiddot That's part of it, but in short, we shouldn't have a buildscript block which is the old notation. For example, classpath("com.android.tools.build:gradle:4.1.0") is not necessary, because the version is set in the settings.gradle.kts like so. Kotlin plugin declaration should also be done in the settings.gradle.kts file similar to this.

Can you please take a look at this a00be91 commit, which resolves the above, and verify that I am not missing anything? 🙇

Cc @fluiddot

oguzkocer · 2022-03-09T17:38:27Z

@ParaskP7 a00be91 looks good to me, although I'd have preferred it if we passed the kotlin version directly instead of relying on a property. In settings.gradle files, using properties can be tricky at times. If it's working as is though, I don't mind keeping it if you prefer it like this.

ParaskP7 · 2022-03-10T08:23:59Z

👋 @oguzkocer !

@ParaskP7 a00be91 looks good to me, although I'd have preferred it if we passed the kotlin version directly instead of relying on a property. In settings.gradle files, using properties can be tricky at times. If it's working as is though, I don't mind keeping it if you prefer it like this.

Thanks for your input, I only chose this way in order to minimize changes from upstream. To be honest I am also preferring if we passed the Kotlin version directly. But yes, it does work as expected. 👍

@fluiddot wdyt, should I do this change or do you prefer us staying as close to the upstream as possible? 🤔

fluiddot · 2022-03-10T08:59:16Z

@fluiddot wdyt, should I do this change or do you prefer us staying as close to the upstream as possible? 🤔

@ParaskP7 Both options work for me, as we're already applying several modifications to the project, making more shouldn't be a problem.

If I understand correctly, the change would be to remove RNGH_kotlinVersion=1.5.20 from android/gradle.properties and set the version directly in android/settings.gradle.kts, right? In that case, I think the change is minor so feel free to apply it.

ParaskP7 · 2022-03-10T09:01:27Z

👋 @fluiddot !

@ParaskP7 Both options work for me, as we're already applying several modifications to the project, making more shouldn't be a problem.

Thanks for your input! 👍

If I understand correctly, the change would be to remove RNGH_kotlinVersion=1.5.20 from android/gradle.properties and set the version directly in android/settings.gradle.kts, right?

Exactly! 👍

In that case, I think the change is minor so feel free to apply it.

On it! 👍

# Conflicts: # react-native-gesture-handler-2.2.0-wp-4.tgz

fluiddot · 2022-03-16T09:15:16Z

@ParaskP7 Heads up that I've updated this branch with the recent changes incorporated in wp-fork (reference) and generated a new tarball. Besides, I also updated the Gutenberg PR to reflect the new version.

ParaskP7 · 2022-03-16T09:30:14Z

👋 @fluiddot !

@ParaskP7 Heads up that I've updated this branch with the recent changes incorporated in wp-fork (#18) and generated a new tarball. Besides, I also updated the WordPress/gutenberg#39097 to reflect the new version.

Thanks for this update! 👍

Btw, I just triggered a Jitpack build based on the latest commit and I got a build failure (see here).

TIL

* What went wrong:
Execution failed for task ':generateDebugRFile'.
> Could not resolve all files for configuration ':debugCompileClasspath'.
   > Could not find com.github.wordpress-mobile:react-native-reanimated:2.4.1-wp-1.
     Required by:
         project :

ParaskP7 · 2022-03-16T09:37:44Z

UPDATE: @fluiddot after your latest merge commit the build is now successful (see here).

PS: Should we also generate a new tarball as well? 🤔

fluiddot · 2022-03-16T09:48:19Z

Btw, I just triggered a Jitpack build based on the latest commit and I got a build failure (see here).

Yep, sorry for that, I forgot to add Jitpack repository, which was required for the latest changes.

UPDATE: @fluiddot after your latest merge commit the build is now successful (see here).

PS: Should we also generate a new tarball as well? 🤔

Right, I've just updated the tarball in aa2bbc9.

ParaskP7 · 2022-04-06T09:32:31Z

Since we removed the source code for these projects from react-native-bridge, we don’t need these updates to happen together with the other projects anymore. That means we can close this draft PR for now and reopen it, if need be, in the future. There is little value in merging this now and diverging from the source repo.

PS: Eventually, we might upgrade Gradle & AGP when using a newer version of React Native if the new RN version requires it.

ParaskP7 added 2 commits February 16, 2022 12:31

Build: Update gradle to 7.3.3

c34ee2a

Build: Update agp to 7.0.4

d2bbed4

ParaskP7 added the enhancement New feature or request label Feb 16, 2022

ParaskP7 requested a review from oguzkocer February 16, 2022 10:35

ParaskP7 self-assigned this Feb 16, 2022

oguzkocer approved these changes Feb 17, 2022

View reviewed changes

ParaskP7 added 4 commits February 24, 2022 15:15

Build: Update gradle to 7.4

006ed46

Build: Update agp to 7.1.1

0cc04ee

Build: Bump version to 1.10.1-wp-5

61f52b7

ParaskP7 changed the title ~~Upgrade Gradle to 7.3.3 & AGP to 7.0.4~~ Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) Feb 24, 2022

fluiddot and others added 10 commits February 25, 2022 12:34

Merge branch 'wp-fork' into update/gradle-to-7.3.3-agp-to-7.0.4

a5b17dc

Generate tarball

eab7a34

Build: Remove old tarball for 1.10.1-wp-4

003a678

Merge branch 'wp-fork' into update/gradle-to-7.3.3-agp-to-7.0.4

2ccfbcc

# Conflicts: # react-native-gesture-handler-1.10.1-wp-5.tgz

Update tarball

3216e18

Merge branch 'wp-fork' of github.com:wordpress-mobile/react-native-ge…

a105a30

…sture-handler into update/gradle-to-7.3.3-agp-to-7.0.4 � Conflicts: � android/gradle.properties � package.json

Build: Bump version to 2.2.0-wp-4

20c7c7b

Build: Generate new tarball for 2.2.0-wp-4

c874463

Build: Update agp to 7.1.1

d424269

Build: Generate new tarball for 2.2.0-wp-4

289bde2

ParaskP7 mentioned this pull request Mar 8, 2022

Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) WordPress/gutenberg#39097

Closed

24 tasks

ParaskP7 added 2 commits March 9, 2022 13:14

Build: Move agp and kotlin dependencies to plugin management

a00be91

Build: Generate new tarball for 2.2.0-wp-4

2d2a860

ParaskP7 mentioned this pull request Mar 9, 2022

Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) wordpress-mobile/react-native-reanimated#9

Closed

ParaskP7 and others added 4 commits March 10, 2022 11:07

Build: Use kotlin version directly instead of a gradle property

7ba53a3

Build: Generate new tarball for 2.2.0-wp-4

1bd872f

Merge branch 'wp-fork' into update/gradle-to-7.3.3-agp-to-7.0.4

27d2395

# Conflicts: # react-native-gesture-handler-2.2.0-wp-4.tgz

Bump version to wp-5

9aa0bb5

Merge branch 'wp-fork' into update/gradle-to-7.3.3-agp-to-7.0.4

9969908

Update tarball

aa2bbc9

ParaskP7 closed this Apr 6, 2022

Conversation

ParaskP7 commented Feb 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ParaskP7 commented Mar 3, 2022

Uh oh!

oguzkocer commented Mar 7, 2022

Uh oh!

fluiddot commented Mar 7, 2022

Uh oh!

oguzkocer commented Mar 7, 2022

Uh oh!

ParaskP7 commented Mar 9, 2022

Uh oh!

oguzkocer commented Mar 9, 2022

Uh oh!

ParaskP7 commented Mar 10, 2022

Uh oh!

fluiddot commented Mar 10, 2022

Uh oh!

ParaskP7 commented Mar 10, 2022

Uh oh!

fluiddot commented Mar 16, 2022

Uh oh!

ParaskP7 commented Mar 16, 2022

Uh oh!

ParaskP7 commented Mar 16, 2022

Uh oh!

fluiddot commented Mar 16, 2022

Uh oh!

ParaskP7 commented Apr 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ParaskP7 commented Feb 16, 2022 •

edited

Loading