Update dependencies 04-2024 #63

smihalic · 2024-04-15T15:23:49Z

📷 Screenshots

No images, but tested this on sample app and some project, no more crashes.

📄 Context

After upgrading AGP to 8.3 a problem appeared with resources sharing the same name, in this case it was menu and string resources. This will happen on all projects that are upgrading and either using this lib or having that problem themselves.

📝 Changes

First of all, resources with same name have been renamed so that is out of way. Next, we used the chance to also bump dependencies.

Desugaring has become somewhat MUST HAVE so I included it in every place compiler was complaining.

In settings.gradle you can see a few optional changes and I commented the reason behind them.

In config.gradle I changed version of sentinel as I was deploying and testing this locally. We should probably return that to 1.3.2, not sure how deployment works.

Tests are failing miserably 😄 . For now, I did something called Pro Developer Move and commented out those failing but when we align on priorities, we can start fixing them. I saw that you used a different approach for this in build.gradle but I couldn't compile it.

📎 Related PR

🚫 Breaking

🛠️ How to test

To test this you have to comment out signing{...} in all publish.gradle files and then run ./gradlew publishReleasePublicationToMavenLocal. I coudn't test this otherwise because I think that duplicated resources are part of last maven deployment.

⏱️ Next steps

We need to align on priorities between fixing tests and removal of desugaring. It seems Chucker also demands desugaring so we might not be able to do this though. Also, toml needs to be included in build.gradle(:sentinel).

Release 1.3.2

Fix readme badges

Fix latest version in readme

KCeh · 2024-04-16T06:21:15Z

First of all, always great to see the initiative 💪

But next time pls contact the library owner (in this case me) before starting work on it. Why? Because I was aware of issue and already made fixes #61 . My solution was in draft because I ran out of time and I actually contacted Google about desugaring changes. Using that as starting point could have save some time.

A few more points:

are you sure gradle has been properly upgraded? I see in my PR AGP upgrade assistant did some changes to build scripts for modules
are you sure you covered all duplicated files? At quick glance in my PR it seems I renamed at least one more file

If you want you can us my PR as starting point. I renamed duplicated resources a bit differently, your approach is probably better.
As for desugaring I will leave comment on that PR

AsimRibo · 2024-04-18T15:21:44Z

settings.gradle

+plugins {
+    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.4.0'
+}
+


We had a conversation about this. I can't resolve toolchains so I have to use this as per documentation. I thought we could commit this maybe until this problem gets resolved [OPTIONAL].

What is a problem with the toolchains?

https://youtrack.jetbrains.com/issue/KTIJ-24981/Gradle-8.-project-sync-fails-with-an-error-No-matching-toolchains-found-for-requested-specification-if-there-is-no-necessary-JDK

AsimRibo · 2024-04-18T15:23:01Z

settings.gradle

@@ -11,3 +15,5 @@ include ":tool-googleplay"
 include ":tool-appgallery"
 include ":tool-thimble"
 include ":tool-timber"
+
+gradle.startParameter.excludedTaskNames.add(":buildSrc:testClasses")


Newest AS has this problem where you can't compile anything. AS is blaming Gradle and vice versa. We have commited this on our project as we are waiting for it to be fixed, so it should be ok here? [OPTIONAL]

So this solves the issue? Test classes won't be compiled?

No, test classes are still compiled 😄 . This only allows you to mitigate this weird bug new AS has.
https://issuetracker.google.com/issues/315023802

AsimRibo · 2024-04-18T15:36:50Z

sentinel/src/androidTest/AndroidManifest.xml

@@ -1,6 +1,5 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
-    package="com.infinum.sentinel"


Package name inside of AndroidManifest is not supported anymore and we were getting errors/warnings, don't remember.

We can set the namespace in build.gradle files.

I think you don't need to set namespace for tests (manifest for tests), gradle will just append .test by default https://developer.android.com/build/configure-app-module#change-namespace-for-testing

antunflas

Consider number of problems, maybe it makes more sense to just release a version without dependency updates, just with resources rename 🤷‍♂️

antunflas · 2024-04-18T16:35:26Z

README.md

@@ -57,15 +57,17 @@ Then add the following dependencies in your app `build.gradle` or `build.gradle.
 **Groovy**

 ```groovy
-debugImplementation "com.infinum.sentinel:sentinel:1.3.1"
-releaseImplementation "com.infinum.sentinel:sentinel-no-op:1.3.1"
+def sentinelVersion = "1.3.2"


As the project already has 2 versions defined, would be good to go in a different direction than this and use a single version everywhere. On some libraries like PrinceOfVersions we have a gradle task to update readme with the current version.

This is correctly updated on master, but it seems we forgot to merge this change on develop. But you are right, this would be an improvement.

I will add idea for Gradle task to the list

antunflas · 2024-04-18T16:36:16Z

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.0.2-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip


8.7 is the latest one, what is the reason to not use that one?

AGP assistant put this one. I will update it to the latest one.

antunflas · 2024-04-18T16:37:22Z

sentinel/src/androidTest/AndroidManifest.xml

@@ -1,6 +1,5 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
-    package="com.infinum.sentinel"


We can set the namespace in build.gradle files.

antunflas · 2024-04-18T16:37:47Z

...c/androidTest/kotlin/com/infinum/sentinel/data/sources/raw/collectors/CollectorsTestSuite.kt

@@ -10,6 +10,6 @@ import org.junit.runners.Suite.SuiteClasses
    DeviceCollectorTestSuite::class,
    ApplicationCollectorTests::class,
    PermissionsCollectorTests::class,
-    PreferencesCollectorTests::class
+//    PreferencesCollectorTests::class


Why is this commented out?

It seems that tests haven't been maintained in a while or some changes were added, I am not in loop. For now I just commented out the problems with them since I don't think I will manage to fix them until Monday. Obviously it won't stay like this, but for now I can't do much since I have project expectations.
I don't want to delete anything since all these tests must pass written conditions once we find time to fix them (soon). I am waiting for Karlo's advice for further steps.

Same for all other comments.

Yeah, tests are causing a lot of issues, compiling sentinel (not sample app) seems to throw errors because of tests.
Some test logic was thrown out a couple of versions ago.
I definitely see values in test, but if they are blocking us we would either need to comment them out (and fix them at later point) or simply remove them

antunflas · 2024-04-18T16:38:04Z

...idTest/kotlin/com/infinum/sentinel/data/sources/raw/collectors/DeviceCollectorDeviceTests.kt

 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
 import com.infinum.sentinel.data.models.raw.DeviceData
+//import com.infinum.sentinel.data.sources.raw.formatters.XmlStringBuilderTests


antunflas · 2024-04-18T16:38:36Z

...Test/kotlin/com/infinum/sentinel/data/sources/raw/collectors/DeviceCollectorEmulatorTests.kt

 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.SmallTest
 import com.infinum.sentinel.data.models.raw.DeviceData
+//import com.infinum.sentinel.data.sources.raw.formatters.XmlStringBuilderTests


Run through the project and remove unused imports please

antunflas · 2024-04-18T16:39:34Z

...c/androidTest/kotlin/com/infinum/sentinel/data/sources/raw/formatters/FormattersTestSuite.kt

+//@RunWith(Suite::class)
+//@SuiteClasses(
+//    PlainStringBuilderTests::class,
+//    MarkdownStringBuilderTests::class,
+//    JsonStringBuilderTests::class,
+//    XmlStringBuilderTests::class,
+//    HtmlStringBuilderTests::class
+//)
+//public class FormattersTestSuite


What is the reason for removing quite many test classes?

antunflas · 2024-04-18T16:40:44Z

settings.gradle

+plugins {
+    id 'org.gradle.toolchains.foojay-resolver-convention' version '0.4.0'
+}
+


What is a problem with the toolchains?

KCeh · 2024-04-19T07:24:52Z

...c/androidTest/kotlin/com/infinum/sentinel/data/sources/raw/collectors/CollectorsTestSuite.kt

@@ -10,6 +10,6 @@ import org.junit.runners.Suite.SuiteClasses
    DeviceCollectorTestSuite::class,
    ApplicationCollectorTests::class,
    PermissionsCollectorTests::class,
-    PreferencesCollectorTests::class
+//    PreferencesCollectorTests::class


Yeah, tests are causing a lot of issues, compiling sentinel (not sample app) seems to throw errors because of tests.
Some test logic was thrown out a couple of versions ago.
I definitely see values in test, but if they are blocking us we would either need to comment them out (and fix them at later point) or simply remove them

KCeh · 2024-04-19T07:25:49Z

sentinel/src/main/res/layout/sentinel_fragment_bundles.xml

@@ -18,7 +18,7 @@
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:minHeight="?attr/actionBarSize"
-            app:menu="@menu/sentinel_bundle_monitor"
+            app:menu="@menu/sentinel_bundle_monitor_menu"


KCeh · 2024-04-19T07:28:00Z

tool-dbinspector/build.gradle

+    compileOptions {
+        coreLibraryDesugaringEnabled true
+    }


I see you added desugaring to each module.
I might be wrong, but it is not only needed for modules that depend on modules that use desugaring (sentinel module, sample app module and timber module?)?

I will take a look at this

Yeah best to recheck if you can but compiler was complaining about desugaring not being enabled in every module.

KCeh · 2024-04-19T07:32:16Z

It seems that CR will fail because of one module using dependency on library from remote repo (new version is not available on remote repo)
https://github.com/infinum/android-sentinel/actions/runs/8740519874/job/23984378145?pr=63
I will look into this

KCeh · 2024-04-19T07:33:04Z

Consider number of problems, maybe it makes more sense to just release a version without dependency updates, just with resources rename 🤷‍♂️

I see your point, but I think 90% of issues are test related. If we remove test we should be fine.

KCeh · 2024-04-19T07:41:03Z

It seems that CR will fail because of one module using dependency on library from remote repo (new version is not available on remote repo) https://github.com/infinum/android-sentinel/actions/runs/8740519874/job/23984378145?pr=63 I will look into this

I see resource changes are all related to Sentinel module. So if we downgrade Sentinel version in libs.version we should be fine.

But,
@antunflas how does this impact library publishing? If in all modules we are always referencing "old" version of core Sentinel module? Again, it won't impact us this time since all changes are in Sentinel module, but it might in future

antunflas · 2024-04-19T07:56:46Z

how does this impact library publishing? If in all modules we are always referencing "old" version of core Sentinel module? Again, it won't impact us this time since all changes are in Sentinel module, but it might in future

You can publish and check the pom.xml file to see what version of a dependency is referenced. Referencing the old Sentinel module is wrong because anyone adding only some tool dependency will get the old version.

I guess the idea with this setup was to:

create a PR with all changes excluding a version change
increment version and deploy to maven
create a PR with the new version

…num/android-sentinel into update-dependencies-04-2024

Quick fix of tests

Only modules that depend on modules that have desugaring enabled should have it

KCeh

Not everything is the best solution (test, need for clients to add desugaring), but for intermediate fix version it will do the job

sonarqubecloud · 2024-04-22T07:17:15Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

KCeh and others added 7 commits March 6, 2024 13:03

Merge pull request #57 from infinum/develop

e22b5de

Release 1.3.2

Fix readme badges

09a6178

Merge pull request #59 from infinum/fix/readme-badges

03fc3fd

Fix readme badges

Fix latest version in readme

141900f

Update versioning in readme

71af394

Merge pull request #60 from infinum/fix/readme-versions

7ad4ec6

Fix latest version in readme

Update dependencies and rename duplicate xml resources

558d44f

smihalic requested review from antunflas, KCeh and AsimRibo April 15, 2024 15:24

smihalic marked this pull request as ready for review April 15, 2024 15:24

KCeh changed the base branch from master to develop April 16, 2024 06:11

KCeh changed the title ~~Update dependencies~~ Update dependencies 04-2024 Apr 16, 2024

KCeh and others added 11 commits April 16, 2024 08:23

Merge branch 'develop' into update-dependencies-04-2024

8464c86

Add plugin and fix for newest AS version to enable running progress work

5677744

Run AGP assistant to update to newest version

d1867ca

Update other dependencies to newest versions

c208281

Add uses feature property for camera permission

d1caa9d

Upgrade ksp version

fdb0fa2

Downgrade kotlin to one version earlier because of ksp warning

333d160

Upgrade sentinel dependencies

ac0ad7b

Remove package name as it is not supported anymore

673a24a

Include core desugaring

82d2f00

Temporarily comment out tests to compile project

885e58c

AsimRibo reviewed Apr 18, 2024

View reviewed changes

antunflas reviewed Apr 18, 2024

View reviewed changes

Use latest gradle distribution

2caeb75

AsimRibo added 2 commits April 18, 2024 22:16

Start of fixing tests

6c2c390

Delete non existent class reference

e5bd2c8

KCeh reviewed Apr 19, 2024

View reviewed changes

Bring down version of sentinel in lib file

c9befa3

Ignore tests

a56cef6

KCeh and others added 2 commits April 19, 2024 10:08

Merge branch 'update-dependencies-04-2024' of https://github.com/infi…

eabfceb

…num/android-sentinel into update-dependencies-04-2024

Merge pull request #64 from infinum/fix-tests-dependencies-04

31b7574

Quick fix of tests

KCeh mentioned this pull request Apr 19, 2024

Update dependencies 03-2024 #61

Closed

KCeh added 4 commits April 19, 2024 10:58

Rename duplicate resource for menu

f1997c9

Revert desugaring changes for modules that don't need them

0e8dbec

Only modules that depend on modules that have desugaring enabled should have it

Fix theming for bottom nav and app bar

0de374f

Return color surface to theme

228d3a0

KCeh requested review from AsimRibo and KCeh April 22, 2024 07:02

KCeh approved these changes Apr 22, 2024

View reviewed changes

Bump version to 1.3.3-alpha01

58526d1

AsimRibo approved these changes Apr 22, 2024

View reviewed changes

KCeh merged commit a9256b8 into develop Apr 22, 2024
6 checks passed

KCeh deleted the update-dependencies-04-2024 branch August 20, 2024 19:46

Update dependencies 04-2024 #63

Update dependencies 04-2024 #63

Conversation

smihalic commented Apr 15, 2024 • edited by AsimRibo Loading

📷 Screenshots

📄 Context

📝 Changes

📎 Related PR

🚫 Breaking

🛠️ How to test

⏱️ Next steps

KCeh commented Apr 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AsimRibo Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AsimRibo Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KCeh Apr 19, 2024 • edited Loading

Choose a reason for hiding this comment

antunflas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KCeh commented Apr 19, 2024

KCeh commented Apr 19, 2024

KCeh commented Apr 19, 2024

antunflas commented Apr 19, 2024

KCeh left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 22, 2024

Quality Gate passed

smihalic commented Apr 15, 2024 •

edited by AsimRibo

Loading

AsimRibo Apr 18, 2024 •

edited

Loading

AsimRibo Apr 19, 2024 •

edited

Loading

KCeh Apr 19, 2024 •

edited

Loading