Skip to content

Upgrade Android Gradle Plugin and Kotlin versions#474

Merged
tonidero merged 17 commits into
mainfrom
upgrade-agp
Jul 20, 2022
Merged

Upgrade Android Gradle Plugin and Kotlin versions#474
tonidero merged 17 commits into
mainfrom
upgrade-agp

Conversation

@vegaro

@vegaro vegaro commented Feb 7, 2022

Copy link
Copy Markdown
Member

From Toni:

Taking over this PR. This will update a bunch of our dependencies versions. None of them should cause breaking changes for our users. Additionally, this will deal with https://revenuecats.atlassian.net/browse/CSDK-334

@aboedo

aboedo commented Jun 10, 2022

Copy link
Copy Markdown
Member

@vegaro what's the status of this one?

@vegaro

vegaro commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

I got stuck with an issue that I couldn't solve and decided to keep it here until I had some time to come back to it. I will rebase and see if it keeps happening and maybe with the help of @tonidero we can upgrade our dependencies

@vegaro

vegaro commented Jun 14, 2022

Copy link
Copy Markdown
Member Author

Alright, I am blocked again @tonidero . Looks like there's an issue with purchase_tester that it's not generating R, but I can't spot it. Locally I have memory issues too. I have created a task in Jira.

This is not high priority at all, but if you want to take a look at it one day that you want to work on something different, feel free to work on this branch. I tend to come back to it on Fridays lol

@tonidero

Copy link
Copy Markdown
Contributor

Sounds good! Will take a look at it when I have some time. Thanks!

@vegaro

vegaro commented Jun 15, 2022

Copy link
Copy Markdown
Member Author

Little update, I managed to fix the test job by fixing purchase tester's namespace. It was causing databinding classes to be generated with the wrong import. It's weird that now the test takes like double the amount of time to run. I suspect is due to memory issues (I get OOM in my machine), so we might need to update the android machine we use in CircleCI.

Integration tests fail for reasons I still don't know...

@tonidero

Copy link
Copy Markdown
Contributor

@vegaro I wanted to ask for an update on this. I was thinking about picking this one up in order to work on https://revenuecats.atlassian.net/browse/CSDK-334 since updating the lifecycle library requires updating the compileVersion, which you're already doing here.

I checked this branch and integration tests are running fine for me. Did you run into any other issues?

@tonidero

Copy link
Copy Markdown
Contributor

Also, tests took < 6min to run for me, which is approximately the same they took before for me.

Comment thread api-tester/build.gradle
maxHeapSize = "1024m"
}
}
namespace 'com.revenuecat.api_tester_kotlin'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm it's weird this is needed now... Afaik, this should be the same as the package of the module by default... Was this due to the R file collision you mentioned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was from an automatic migration to the latest AGP version. Maybe it's not mandatory?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh interesting... Maybe it's needed if applicationId is not set? I thought it would automatically pick the package name and use that as the namespace but it's possible you need to set one or the other. I will take another look. In any case, I'm ok with this change!

Comment thread build.gradle Outdated
plugins {
id "io.gitlab.arturbosch.detekt" version "1.17.0-RC3"
id "com.savvasdalkitsis.module-dependency-graph" version "0.9"
id "com.github.ben-manes.versions" version "0.42.0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was wondering how are you planning to use this plugin? Looks like it can be a useful tool to keep things updated, so not sure if we should add it's tasks somewhere...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was thinking about including some sort of automation that automatically upgrades dependencies instead of this plugin. I used this one to fix a couple issues I was having but I don't think we have to merge it into main

Comment thread integration-tests/build.gradle Outdated

android {
compileSdkVersion 30
compileSdkVersion 31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can probably use the compileVersion here? Not sure if we want to link it to the sdk, but I think that's ok?

Comment thread purchases/build.gradle Outdated
unityIAPApi "com.android.billingclient:billing:$billingClient3Version"
implementation "androidx.lifecycle:lifecycle-runtime:$lifecycleVersion"
kapt "androidx.lifecycle:lifecycle-compiler:$lifecycleVersion"
implementation "androidx.lifecycle:lifecycle-common-java8:$lifecycleVersion"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe you can remove the -java8 suffix, since everything was moved to the lifecycle-common dependency

Comment thread examples/MagicWeather/build.gradle Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We are not using the common build.gradle for the MagicWeather app, so I had to duplicate the versions here... We could have tried to import it, but considering that one has things related to the SDK, it felt better to keep it separate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah I think it's because it's not part of the project

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread examples/MagicWeather/build.gradle Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Latest is actually 1.7.10. I'm planning to merge this and update it further on future PRs.

@tonidero tonidero marked this pull request as ready for review July 20, 2022 10:28
@tonidero tonidero requested a review from a team July 20, 2022 10:28

@vegaro vegaro left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I can't approve it because I am the owner lol but feel free to approve it yourself @tonidero . It looks good

@tonidero tonidero merged commit 3852820 into main Jul 20, 2022
@tonidero tonidero deleted the upgrade-agp branch July 20, 2022 11:57
@tonidero tonidero mentioned this pull request Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants