Bug 1870050 - Upgrade AGP and kotlin dsl version. #4797

mcarare · 2023-12-11T16:51:31Z

Pull Request checklist

Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
Tests: This PR includes thorough tests or an explanation of why it does not
Changelog: This PR includes a changelog entry or does not need one
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

Breaking Changes: If this is a breaking Android Components change, please push a draft PR on Reference Browser to address the breaking issues.

To download an APK when reviewing a PR (after all CI tasks finished running):

Click on Checks at the top of the PR page.
Click on the firefoxci-taskcluster group on the left to expand all tasks.
Click on the build-apk-{fenix,focus,klar}-debug task you're interested in.
Click on View task in Taskcluster in the new DETAILS section.
The APK links should be on the right side of the screen, named for each CPU architecture.

GitHub Automation

https://bugzilla.mozilla.org/show_bug.cgi?id=1870050

mcarare · 2023-12-11T16:51:41Z

🧪

jonalmeida

These changes look fine to me on the surface. I haven't run it locally, but nothing here looks suspicious enough to warrant that.

jonalmeida · 2024-01-31T21:20:02Z

android-components/plugins/config/build.gradle

@@ -3,7 +3,7 @@
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

 plugins {
-    id "org.gradle.kotlin.kotlin-dsl" version "4.1.2"
+    id "org.gradle.kotlin.kotlin-dsl" version "4.2.1"


Woohoo! Goodbye warnings!

Is there a way to move this version to the DependenciesPlugin or catalogs in the future? I wouldn't change your PR today, maybe in a future revision so we can land this sooner.

jonalmeida · 2024-01-31T21:22:09Z

fenix/app/src/main/java/org/mozilla/fenix/components/FenixSnackbar.kt

@@ -121,7 +121,7 @@ class FenixSnackbar private constructor(
         * Suppressing ComplexCondition. Yes it's unfortunately complex but that's the nature
         * of the snackbar handling by Android. It will be simpler once dynamic toolbar is always on.
         */
-        @Suppress("ComplexCondition")
+        @Suppress("ComplexCondition", "RestrictedApi")


Should we file an issue to remove access to what is causing this restricted API?

Same question for the one in fenix/app/src/main/java/org/mozilla/fenix/ext/Toolbar.kt too.

https://bugzilla.mozilla.org/show_bug.cgi?id=1877983

jonalmeida · 2024-01-31T21:33:09Z

focus-android/app/src/main/AndroidManifest.xml

@@ -180,11 +180,13 @@
        <service
            android:name=".downloads.DownloadService"
            android:exported="false"
-            android:foregroundServiceType="dataSync" />
+            android:foregroundServiceType="dataSync"
+            tools:ignore="ForegroundServicePermission" />


Why do we ignore this here? Is it because we request this permission at runtime? If so, can we leave that as a note here: "Permission requested at runtime."

This looks like a false positive because the required permission is in the merged manifest and not in the Focus manifest. An alternative would be adding the permissions explicitly.

I guess it would be better for future references to have the permissions declared explicitly instead of having suppressions ( that would likely be fixed in future lint releases)

Also sync lint version.

Skipping this parameter would result in a UnspecifiedRegisterReceiverFlag error.

The permissions already included the merged manifest, but linting shows error if not added directly.

jonalmeida · 2024-02-01T22:28:09Z

lgtm!

mcarare · 2024-02-02T06:54:14Z

Merging this on Monday.

github-actions bot added the work in progress Not ready to land yet. Work in progress (WIP). label Dec 11, 2023

mozilla-mobile deleted a comment from github-actions bot Dec 12, 2023

mcarare force-pushed the agp branch from 42dc484 to 715f377 Compare December 12, 2023 11:17

mozilla-mobile deleted a comment from github-actions bot Dec 12, 2023

mcarare force-pushed the agp branch 3 times, most recently from c9dfc79 to 4994045 Compare December 14, 2023 14:51

mcarare changed the title ~~Upgrade AGP and kotlin dsl version.~~ Bug 1870050 - Upgrade AGP and kotlin dsl version. Dec 14, 2023

mozilla-mobile deleted a comment from github-actions bot Dec 14, 2023

mcarare force-pushed the agp branch 3 times, most recently from c433151 to 7bb1c4a Compare December 15, 2023 13:35

mcarare force-pushed the agp branch 2 times, most recently from f73c063 to 84e35cd Compare December 28, 2023 11:47

mcarare force-pushed the agp branch 4 times, most recently from a9286ae to f7b9821 Compare January 25, 2024 15:35

mcarare marked this pull request as ready for review January 31, 2024 12:17

github-actions bot added 🕵️‍♀️ needs review PRs that need to be reviewed and removed work in progress Not ready to land yet. Work in progress (WIP). labels Jan 31, 2024

jonalmeida approved these changes Jan 31, 2024

View reviewed changes

mcarare force-pushed the agp branch from 02d0f84 to fcd26ce Compare February 1, 2024 12:09

github-actions bot assigned jonalmeida Feb 1, 2024

github-actions bot removed the 🕵️‍♀️ needs review PRs that need to be reviewed label Feb 1, 2024

github-actions bot added the approved PR that has been approved label Feb 1, 2024

mcarare force-pushed the agp branch from fcd26ce to 56051dd Compare February 1, 2024 12:12

mcarare added 7 commits February 1, 2024 19:02

Bug 1870050 - Upgrade AGP and kotlin dsl version.

1e8c9a1

Also sync lint version.

Bug 1870050 - Suppress RestrictedApi lint

c56328d

Bug 1870050 - Suppress false positive DiffUtilEquals lint

736a16a

Bug 1870050 - Remove improper usage of VisibleForTesting annotation

aa4be7f

Bug 1870050 - Add check for proper RegisterReceiverFlags in tests.

0d75ef0

Skipping this parameter would result in a UnspecifiedRegisterReceiverFlag error.

Bug 1870050 - Explicitly add FOREGROUND_SERVICE specific permissions.

eb21d46

The permissions already included the merged manifest, but linting shows error if not added directly.

Bug 1870050 - Add RECEIVER_NOT_EXPORTED flag to receiver.

6d73bee

mcarare force-pushed the agp branch from 56051dd to 6d73bee Compare February 1, 2024 17:02

mcarare added the 🛬 needs landing PRs that are ready to land label Feb 5, 2024

Merge branch 'main' into agp

6a31e73

mergify bot merged commit 5ab7a32 into mozilla-mobile:main Feb 5, 2024
298 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1870050 - Upgrade AGP and kotlin dsl version. #4797

Bug 1870050 - Upgrade AGP and kotlin dsl version. #4797

mcarare commented Dec 11, 2023 •

edited by github-actions bot

Loading

mcarare commented Dec 11, 2023

jonalmeida left a comment

jonalmeida Jan 31, 2024

jonalmeida Jan 31, 2024

mcarare Feb 1, 2024

jonalmeida Jan 31, 2024

mcarare Feb 1, 2024

mcarare Feb 1, 2024

jonalmeida commented Feb 1, 2024

mcarare commented Feb 2, 2024

Bug 1870050 - Upgrade AGP and kotlin dsl version. #4797

Bug 1870050 - Upgrade AGP and kotlin dsl version. #4797

Conversation

mcarare commented Dec 11, 2023 • edited by github-actions bot Loading

Pull Request checklist

After merge

To download an APK when reviewing a PR (after all CI tasks finished running):

GitHub Automation

mcarare commented Dec 11, 2023

jonalmeida left a comment

Choose a reason for hiding this comment

jonalmeida Jan 31, 2024

Choose a reason for hiding this comment

jonalmeida Jan 31, 2024

Choose a reason for hiding this comment

mcarare Feb 1, 2024

Choose a reason for hiding this comment

jonalmeida Jan 31, 2024

Choose a reason for hiding this comment

mcarare Feb 1, 2024

Choose a reason for hiding this comment

mcarare Feb 1, 2024

Choose a reason for hiding this comment

jonalmeida commented Feb 1, 2024

mcarare commented Feb 2, 2024

mcarare commented Dec 11, 2023 •

edited by github-actions bot

Loading