ci: bring :app and :e2eTest under the Android Lint gate - #2500
Conversation
There was a problem hiding this comment.
Code Review
This pull request configures Android Lint for the :app and :e2eTest modules, updates CI documentation to reflect that all 10 modules are now gated, cleans up unused resources, and adds a monochrome layer to the adaptive launcher icons. Feedback points out that the <monochrome> layer incorrectly references a raster image instead of a vector drawable, which will break themed icons on Android 13+. Additionally, it is recommended to centralize the duplicated lint configurations across modules to adhere to DRY principles.
| <adaptive-icon xmlns:android="http://schemas.android.com/apk/res/android"> | ||
| <background android:drawable="@color/ic_launcher_background"/> | ||
| <foreground android:drawable="@mipmap/ic_launcher_foreground"/> | ||
| <monochrome android:drawable="@mipmap/ic_launcher_foreground"/> |
There was a problem hiding this comment.
The <monochrome> layer of an adaptive icon must be a vector drawable so that the system can dynamically tint it according to the user's theme. Since @mipmap/ic_launcher_foreground is a webp (raster) image, using it here will prevent themed icons from rendering correctly on Android 13+ devices. Consider providing a flat vector drawable for the monochrome layer instead.
| <adaptive-icon xmlns:android="http://schemas.android.com/apk/res/android"> | ||
| <background android:drawable="@color/ic_launcher_background"/> | ||
| <foreground android:drawable="@mipmap/ic_launcher_foreground"/> | ||
| <monochrome android:drawable="@mipmap/ic_launcher_foreground"/> |
There was a problem hiding this comment.
The <monochrome> layer of an adaptive icon must be a vector drawable so that the system can dynamically tint it according to the user's theme. Since @mipmap/ic_launcher_foreground is a webp (raster) image, using it here will prevent themed icons from rendering correctly on Android 13+ devices. Consider providing a flat vector drawable for the monochrome layer instead.
| lint { | ||
| // Common lint options across all modules | ||
| disable += mutableSetOf( | ||
| "IconExpectedSize", | ||
| "InvalidPackage", // Firestore uses GRPC which makes lint mad | ||
| "NewerVersionAvailable", "GradleDependency", // For reproducible builds | ||
| "SelectableText", "SyntheticAccessor" // We almost never care about this | ||
| ) |
There was a problem hiding this comment.
The common lint options are duplicated across multiple modules (e.g., :app and :e2eTest). This violates the DRY (Don't Repeat Yourself) principle and increases maintenance overhead when adding or removing global lint rules. Consider centralizing these common lint options in the root build.gradle.kts file using subprojects or by creating a custom Gradle convention plugin.
| lint { | ||
| // Common lint options across all modules | ||
| disable += mutableSetOf( | ||
| "IconExpectedSize", | ||
| "InvalidPackage", // Firestore uses GRPC which makes lint mad | ||
| "NewerVersionAvailable", "GradleDependency", // For reproducible builds | ||
| "SelectableText", "SyntheticAccessor" // We almost never care about this | ||
| ) |
There was a problem hiding this comment.
The common lint options are duplicated across multiple modules (e.g., :app and :e2eTest). This violates the DRY (Don't Repeat Yourself) principle and increases maintenance overhead when adding or removing global lint rules. Consider centralizing these common lint options in the root build.gradle.kts file using subprojects or by creating a custom Gradle convention plugin.
9fcd7f0 to
0d104b2
Compare
0027ad3 to
b18edc9
Compare
64ce7cf to
4f2b154
Compare
b18edc9 to
5b326ae
Compare
No description provided.