fix: update plugin gradle script #5578

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

Jump to bottom

Open

farfromrefug wants to merge 4 commits into NativeScript:main

from Akylas:android_gradle_script

Open

fix: update plugin gradle script #5578

farfromrefug wants to merge 4 commits into NativeScript:main from Akylas:android_gradle_script

Conversation

farfromrefug

Copy link

Contributor

@farfromrefug farfromrefug commented Sep 26, 2021

This updates the plugin gradle script:

no more android support, use androidx
use java 1.8 (removes warning in the console
koltin, androidx versions through project properties

@farfromrefug


 fix: update plugin gradle script

d0556f0

@cla-bot cla-bot bot added the cla: yes label

Sep 26, 2021

@rigor789

Copy link

Member

rigor789 commented Sep 30, 2021

no more android support, use androidx

Would that be considered a BREAKING CHANGE?

@farfromrefug

Copy link

Contributor Author

farfromrefug commented Sep 30, 2021

Do we still support old versions without androidx?

@rigor789

Copy link

Member

rigor789 commented Sep 30, 2021

I'm not sure - some plugins may still use it?

@farfromrefug

Copy link

Contributor Author

farfromrefug commented Sep 30, 2021

Seems unlikely. And anyway they can deps on their include.gradle.
Honestly we should remove them, plus they end up in the .dat metadata...

@rigor789

Copy link

Member

rigor789 commented Sep 30, 2021

👍 - just don't want to break things by accident in a patch release

@farfromrefug

Copy link

Contributor Author

farfromrefug commented Sep 30, 2021

@rigor789 i think it should got with android runtime 8.2.0 as we move to gradle 7. But i understand your point

farfromrefug added 3 commits

October 4, 2021 12:10

@farfromrefug


 chore: refactoring

3c716f7

@farfromrefug


 fix: makes libs smaller and build faster

40b5eb8

see https://www.mobileit.cz/Blog/Pages/r-class.aspx

@farfromrefug


 fix: remove unneeded R.class and BuildConfig.class for plugins

c4970e5

can still be enabled with `generateBuildConfig` and `generateR`

@farfromrefug

Copy link

Contributor Author

farfromrefug commented Oct 4, 2021

@rigor789 @triniwiz that PR now goes further and makes generated plugins aar like 10 times smaller. The consequence is apps dex files getting also much smaller (mine dont even need multidex anymore)

@triniwiz

Copy link

Member

triniwiz commented Oct 4, 2021

All support packages would be jetpacked (if you wanna call it that) .. this looks good for 8.2

@rigor789 rigor789 added this to the 8.2 milestone

Nov 23, 2021

rigor789

rigor789 reviewed

Feb 15, 2022

View reviewed changes

Copy link

Member

@rigor789 rigor789 left a comment

Choose a reason for hiding this comment

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

@farfromrefug @triniwiz if you could help clear up questions I posted in the review, that would help a ton with landing this.

vendor/gradle-plugin/build.gradle

apply plugin: 'kotlin-android'

apply plugin: 'kotlin-android-extensions'

def computeKotlinVersion = { -> project.hasProperty("kotlinVersion") ? kotlinVersion : "1.4.21" }

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Can we bump kotlinVersion without too much testing, or is 1.4.21 safer for now?

vendor/gradle-plugin/build.gradle

apply plugin: 'kotlin-android-extensions'

def computeKotlinVersion = { -> project.hasProperty("kotlinVersion") ? kotlinVersion : "1.4.21" }

def computeCompileSdkVersion = { -> project.hasProperty("compileSdk") ? compileSdk : 28 as int }

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Should we change the default compileSdk from 28 to 30 (or perhaps newer)?

vendor/gradle-plugin/build.gradle

def computeKotlinVersion = { -> project.hasProperty("kotlinVersion") ? kotlinVersion : "1.4.21" }

def computeCompileSdkVersion = { -> project.hasProperty("compileSdk") ? compileSdk : 28 as int }

def computeTargetSdkVersion = { -> project.hasProperty("targetSdk") ? targetSdk : 28 as int }

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Should we change targetSdk to 30 or newer? Believe 30 is minimum from google right now.

vendor/gradle-plugin/build.gradle

def computeCompileSdkVersion = { -> project.hasProperty("compileSdk") ? compileSdk : 28 as int }

def computeTargetSdkVersion = { -> project.hasProperty("targetSdk") ? targetSdk : 28 as int }

def computeBuildToolsVersion = { ->

project.hasProperty("buildToolsVersion") ? buildToolsVersion : "28.0.3" as String

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

With 8.2 - should we default to build tools 32?

vendor/gradle-plugin/build.gradle

android {

kotlinOptions {

jvmTarget = '1.8'

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Do we need to set the target to 1.8 with gradle7+? Officially recommended jdk is 11, so this might need to match that?

Copy link

Contributor Author

@farfromrefug farfromrefug Feb 15, 2022

Choose a reason for hiding this comment

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

we should be able to use more but was not tested https://kotlinlang.org/docs/faq.html#which-versions-of-jvm-does-kotlin-target
let s make it an ext property so we can test easily?

vendor/gradle-plugin/build.gradle

Comment on lines +107 to +108

sourceCompatibility JavaVersion.VERSION_1_8

targetCompatibility JavaVersion.VERSION_1_8

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Same question as jvmTarget

vendor/gradle-plugin/build.gradle

}

def androidXMultidexVersion = project.hasProperty("androidXMultidexVersion") ? project.androidXMultidexVersion : "2.0.1"

def androidxVersion = project.hasProperty("androidxVersion") ? project.androidxVersion : "1.2.0"

Copy link

Member

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

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

Can we use 1.7.0?

Copy link

Contributor Author

@farfromrefug farfromrefug Feb 15, 2022

Choose a reason for hiding this comment

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

not sure about 1.7.0. Maybe 1.6.0

vendor/gradle-plugin/build.gradle Show resolved Hide resolved

@rigor789 rigor789 removed this from the 8.2 milestone

Jul 25, 2022

@madmas madmas mentioned this pull request

Jan 24, 2023

CLI 8.2.x cannot build plugin with targetSdk 32 and buildTools 32.0.0 #5660

Open

3 tasks

@farfromrefug

Copy link

Contributor Author

farfromrefug commented Jan 17, 2024

superseded by #5774

Labels

cla: yes

3 participants

@farfromrefug @rigor789 @triniwiz

Uh oh!

fix: update plugin gradle script #5578

Are you sure you want to change the base?

fix: update plugin gradle script #5578

Uh oh!

Conversation

@farfromrefug farfromrefug commented Sep 26, 2021

Uh oh!

rigor789 commented Sep 30, 2021

Uh oh!

farfromrefug commented Sep 30, 2021

Uh oh!

rigor789 commented Sep 30, 2021

Uh oh!

farfromrefug commented Sep 30, 2021

Uh oh!

rigor789 commented Sep 30, 2021

Uh oh!

farfromrefug commented Sep 30, 2021

Uh oh!

farfromrefug commented Oct 4, 2021

Uh oh!

triniwiz commented Oct 4, 2021

Uh oh!

@rigor789 rigor789 left a comment

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@farfromrefug farfromrefug Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@rigor789 rigor789 Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

@farfromrefug farfromrefug Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

farfromrefug commented Jan 17, 2024

Uh oh!

Uh oh!