Run Android linter; get our tree clean of those lints #797
Conversation
|
Looks like something is missing in the CI testing environment. Quoting the test failure: |
|
Yeah, fixing now |
|
The fix (I think — we'll see if this CI run passes) was: + # This causes `android/gradlew` to exist, for `tools/gradle` to use.
+ flutter build apk --config-only \
+ || return
+
# For docs on this Android linter:
# https://developer.android.com/studio/write/lint
tools/gradle -q :app:lint \ |
There was a problem hiding this comment.
Cool! LGTM assuming CI passes. I'll hold off merging until @rajveermalviya has a chance to look, as I did in #796.
| @@ -1,7 +1,7 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <!-- Modify this file to customize your launch splash screen --> | |||
| <layer-list xmlns:android="http://schemas.android.com/apk/res/android"> | |||
| <item android:drawable="@android:color/white" /> | |||
| <item android:drawable="?android:colorBackground" /> | |||
There was a problem hiding this comment.
android [nfc]: Merge drawable-v21/ into drawable/
Our minSdkVersion is no less than 21 -- in fact it's 28 -- so the
app only ever runs on devices that will pick the v21 version over
the baseline version. So simplify that variation away.
This may break local Android builds in an existing worktree;
it seems like the Android Gradle Plugin is inappropriately caching
where it expects to find this file and gets confused when that copy
goes missing. The error looks like this:
* What went wrong:
Execution failed for task ':app:processDebugResources'.
> A failure occurred while executing com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$TaskAction
> Android resource linking failed
com.zulip.flutter.app-mergeDebugResources-32:/values/values.xml:213: error: resource drawable/launch_background (aka com.zulip.flutter:drawable/launch_background) not found.
[…]
To fix, clear the relevant cached data by running:
$ tools/gradle -q --rerun-tasks \
:app:bundleDebugResources :app:bundleReleaseResources
Cool, that fix worked. My error was slightly different: it was talking about "release resources" instead of "debug resources", because tools/check android apparently makes a release build. Is it intended to make a release build, by the way?
There was a problem hiding this comment.
Yeah, it'll be "debug resources" if e.g. you hit this as part of flutter run (and I guess don't pass --release or --profile).
I think a release build is a bit more important to exercise here than a debug build, because if there's some issue that one of them trips but not the other, then the release build is much more likely to give us a belated surprise.
There was a problem hiding this comment.
Thanks LGTM!
Same as the previous Android build PR (#796 (review)), there isn't any difference when M1 runner is used: https://github.com/rajveermalviya/zulip-flutter/actions/runs/9854961236/job/27208869364
Which makes me believe most of the time is spent in dependency downloads, that could be cached, not a reason to block this PR though.
This was introduced in cffb112 / zulip#474 because we thought it would be a fix for zulip#461. As discussed on that issue thread, though, it turns out this file doesn't have an effect on the Dart HTTP implementation, which we use. Since it looks like it's doing something but in fact it isn't, cut it out to avoid confusion. It'll still be there in the Git history if we later want to use it as part of a future effort to fix zulip#461.
Our minSdkVersion is no less than 21 -- in fact it's 28 -- so the
app only ever runs on devices that will pick the v21 version over
the baseline version. So simplify that variation away.
This may break local Android builds in an existing worktree;
it seems like the Android Gradle Plugin is inappropriately caching
where it expects to find this file and gets confused when that copy
goes missing. The error looks like this:
* What went wrong:
Execution failed for task ':app:processDebugResources'.
> A failure occurred while executing com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$TaskAction
> Android resource linking failed
com.zulip.flutter.app-mergeDebugResources-32:/values/values.xml:213: error: resource drawable/launch_background (aka com.zulip.flutter:drawable/launch_background) not found.
[…]
To fix, clear the relevant cached data by running:
$ tools/gradle -q --rerun-tasks \
:app:bundleDebugResources :app:bundleReleaseResources
|
Thanks for the reviews! Merging. |
Stacked atop #796.
This adds the Android linter:
https://developer.android.com/studio/write/lint
to what we run in CI.
Then it turns out there are a couple of things that linter finds in our existing tree. So fix those first.