Skip to content
This repository was archived by the owner on Mar 18, 2022. It is now read-only.

Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier & Min SDK Version)#2

Closed
ParaskP7 wants to merge 9 commits intowp-forkfrom
update/gradle-to-7.3.3-agp-to-7.0.4
Closed

Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier & Min SDK Version)#2
ParaskP7 wants to merge 9 commits intowp-forkfrom
update/gradle-to-7.3.3-agp-to-7.0.4

Conversation

@ParaskP7
Copy link
Copy Markdown

@ParaskP7 ParaskP7 commented Feb 16, 2022 •
edited
Loading

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).
  • Version bump to 1.9.0-wp-2 (see package.json).
  • Tarball update to 1.9.0-wp-2 (see react-native-clipboard-clipboard-1.9.0-wp-2.tgz).
  • The build was also verified through Jitpack (see build.log).

This PR also included the following changes:

  • Update min SKD 21 (see android/gradle.properties change).

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.

@ParaskP7 ParaskP7 added the enhancement New feature or request label Feb 16, 2022
@ParaskP7 ParaskP7 requested a review from oguzkocer February 16, 2022 11:25
@ParaskP7 ParaskP7 self-assigned this Feb 16, 2022
@ParaskP7
Build: Update gradle to 7.4
18845f4
@ParaskP7
Build: Update agp to 7.1.1
92abe5f
@ParaskP7
Build: Disable jetifier
97ad2d0 
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

===================================================== ...
* No dependencies with legacy android support usages! ...
===================================================== ...

... ===============================
... You can say Bye Bye Jetifier. *
... ===============================
@ParaskP7
Build: Bump version to 1.9.0-wp-2
da4c0ea
@ParaskP7 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
@ParaskP7
Build: Update min sdk version to 21
b09baa4 
This was done because Lint otherwise fails with the below message:

"uses-sdk:minSdkVersion 16 cannot be smaller than version 21 declared
 in library [com.facebook.react:react-native:0.64.0] /Users/<user>/
 .gradle/caches/transforms-3/49f36d12f575c939d228607cd5223ea8/
 transformed/react-native-0.64.0/AndroidManifest.xml as the library
 might be using APIs not available in 16"

The 'minSdkVersion=21' is used because all the other node module fork
libraries are also using the same version and as such this matches with
everything else.
@ParaskP7 ParaskP7 changed the title Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier) Upgrade Gradle to 7.4 & AGP to 7.1.1 (& Disable Jetifier & Min SDK Version) Feb 24, 2022
@ParaskP7
Build: Add skip lib check as true into ts config json file
c13094c 
This solves the issue with 'npm install && npm pack' not being able to
generate a new tarball.

For more info you can reference this comment: https://github.com/
react-native-community/react-native-template-typescript/
issues/203#issuecomment-946254950
@ParaskP7
Build: Generate new tarball for 1.9.0-wp-2
773d150
@ParaskP7
Copy link
Copy Markdown
Author

ParaskP7 commented Feb 25, 2022 •
edited
Loading

👋 @hypest !

Can you verify that this change I did here, which updates the minSdkVersion to 21, is actually correct? 🙇

PS: I did that because otherwise Lint fails for both, this repo and the react-native-bridge module within the gutenberg repo.

@hypest
Copy link
Copy Markdown

hypest commented Feb 25, 2022

Can you verify that this change I did here, which updates the minSdkVersion to 21, is actually correct? 🙇

Happy to help @ParaskP7 ! Can you share the GB PR or the WPAndroid PR to test it out? Thanks!

@ParaskP7
Copy link
Copy Markdown
Author

ParaskP7 commented Feb 25, 2022

Happy to help @ParaskP7 ! Can you share the GB PR or the WPAndroid PR to test it out? Thanks!

Thank you @hypest ! You can test by running Lint on this repo and see the outcome if you revert my change from 21 to 16. As far as testing it, with either a GB PR or WPAndroid PR, unfortunately those are not ready yet. 🤔

PS: I am currently working on creating the GB PR. I hope this will help you with your testing. I'll let you know when it gets available. 👍

@hypest
Copy link
Copy Markdown

hypest commented Feb 25, 2022

Thanks for the testing info @ParaskP7 , I verified it on my side that lint is not complaining with the 16->21 bump.

I wonder if this is a change we should push upstream 🤔 , wdyt?

@ParaskP7
Copy link
Copy Markdown
Author

ParaskP7 commented Feb 25, 2022 •
edited
Loading

Thanks for the testing info @ParaskP7 , I verified it on my side that lint is not complaining with the 16->21 bump.

Thanks for verifying that @hypest ! 🙇

I wonder if this is a change we should push upstream 🤔 , wdyt?

As I explained on my commit message, since all other such forked repos are already pointing to a minSdkVersion of 21 instead of 16, we should at least match that expectation on the node module folk libraries level. I am also seeing all the gutenberg Android modules are pointing to minSdkVersion of 21.

In addition to that, on the WPAndroid side, we are looking at a minSdkVersion of 24, which mean that at least that client would no longer support minSdkVersion of 21, let along minSdkVersion of 16. All that makes me think that we should drop support for minSdkVersion of 16 for this repo and actually later on we might want to even drop support for minSdkVersion of 21 in favor of minSdkVersion of 24. Wdyt? 🤔

@hypest
Copy link
Copy Markdown

hypest commented Feb 25, 2022

Verified that the lint error is related to the gradle and AGP update anyway so, pushing the minSDK change upstream would also require pushing all the android build diff so far, and I think that's outside the scope for the effort here. So, I'm cool from my side to contain the changes here.

@ParaskP7
Copy link
Copy Markdown
Author

ParaskP7 commented Feb 25, 2022

Verified that the lint error is related to the gradle and AGP update anyway so, pushing the minSDK change upstream would also require pushing all the android build diff so far, and I think that's outside the scope for the effort here. So, I'm cool from my side to contain the changes here.

So, just to get this right @hypest you too think that this change of minSdkVersion, from 16, to 21, most probably shouldn't cause any problems to GB or its clients, like WPAndroid, so this is a 👍 to proceed, right?

PS: We will anyway be testing GB and WPAndroid extensively afterwards to verify that all these changes are okay. If, during out testing, we will find issue related to that we can revert it anyway.

@hypest
Copy link
Copy Markdown

hypest commented Feb 25, 2022

Oh, yeah, I don't expect the minSDKVersion bump to 21 to be a problem for Gutenberg, especially since it's already the minimum for RN itself. Besides, as you mention, we'll try it more via the WPAndroid and Gutenberg PRs.

@ParaskP7 ParaskP7 mentioned this pull request Mar 8, 2022
Closed
24 tasks
@hypest
Copy link
Copy Markdown

hypest commented Mar 18, 2022

We are retiring this repo so, no need for further work on this PR. Thank you for the hard work on this this far folks! ❤️

@hypest hypest closed this Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@oguzkocer oguzkocer oguzkocer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ParaskP7 ParaskP7

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ParaskP7 @hypest @oguzkocer