Skip to content

Do Android build in CI #796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2024
Merged

Do Android build in CI #796

merged 3 commits into from
Jul 10, 2024

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jul 8, 2024

Fixes #772.

(Started as a draft PR, because I wasn't sure if more setup was needed in ci.yml in order for this build to work. Indeed some was.)

@gnprice gnprice force-pushed the pr-android-ci branch 3 times, most recently from a185d56 to a618e33 Compare July 8, 2024 22:52
@gnprice gnprice marked this pull request as ready for review July 8, 2024 23:01
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! I'll hold off merging until @rajveermalviya has a chance to look.

@gnprice
Copy link
Member Author

gnprice commented Jul 8, 2024

The other day I remarked:
#718 (comment)
that when we handle #772, we should include running the linter. It turns out that the type of issue that we were discussing there was a warning from the Kotlin compiler — so the last commit in this PR, telling the Kotlin compiler to treat those warnings as errors, does the job.

There's also an Android linter, but it has a different job. I'll send a separate PR for running that.

Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @gnprice, LGTM!

I tried this with macOS M1 runner (macos-latest), but surprisingly it didn't make much difference: https://github.com/rajveermalviya/zulip-flutter/actions/runs/9852479999

@gnprice
Copy link
Member Author

gnprice commented Jul 10, 2024

I tried this with macOS M1 runner (macos-latest), but surprisingly it didn't make much difference: https://github.com/rajveermalviya/zulip-flutter/actions/runs/9852479999

Thanks for trying that. Yeah, it's a shame that this adds so much to the time CI requires — about 4 minutes. It'd be good to find a way to reduce that, but I'm not sure there will be a good way; I think building an Android app is just slow compared to what we do in flutter test, and the worker machines we get from GitHub Actions are low-powered, and so on net it's slow.

gnprice added 3 commits July 9, 2024 21:04
And give it the necessary prerequisites in CI.  By default the
build worker supplies JDK 11, apparently, and that's too old.
In particular this applies to the new `tools/check android`.

A StackOverflow question that had a couple of helpful answers:
  https://stackoverflow.com/questions/34562200/how-do-i-make-the-kotlin-compiler-treat-warnings-as-errors
@gnprice gnprice merged commit b2def8e into zulip:main Jul 10, 2024
1 check passed
@gnprice gnprice deleted the pr-android-ci branch July 10, 2024 04:06
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.

Run Android build in CI
3 participants