Conversation
|
How do we intend to deploy the web version for the Kotlin Multiplatform @qiarie? Can you trigger a sample release for purposes of testing we can delete the tag afterwards. |
Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com>
Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com>
Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com>
Let callers pass values explicitly
- Closed shell-injection sink in preflight - Hardened env-var handling (VERSION_NAME/VERSION_CODE blank-safe - composePackageVersion fails loudly on malformed input) - Widened tag regex to Semantic Versioning-legal hyphen suffixes - Added count-check to the desktop matrix output - Updated spotless job fetch-depth: 0 so ratchetFrom = "origin/main" resolves
actions/checkout only populates the ref it checks out — refs/pull/<N>/merge for PRs - so refs/remotes/origin/main is never created and spotless' ratchetFrom = "origin/main" fails at task-graph construction. Switch the spotless job to fetch-depth: 0 and add an explicit fetch that creates the missing ref
- Upgrade Kotlin to 2.3.21 - Disable unstable JS and Android-host test tasks
Refer to this test pre-release Release v1.0.0-test.4 |
| # Each leg gets its own 16 GB runner, so the JS and Wasm test-executable | ||
| # compiles don't compete with each other for Kotlin daemon heap. |
| # compileTestDevelopmentExecutableKotlinJs has historically OOMed | ||
| # at 6g; give the Kotlin daemon more room only for this leg. |
There was a problem hiding this comment.
Delete comment. This can be inferred from the setting.
| tasks: >- | ||
| :ohs-player-library:assembleAndroidMain | ||
| :ohs-player-reference-app:lintDebug |
There was a problem hiding this comment.
What's the intention for these two tasks? They serve different purposes. The assembleAndroidMain should be used to compile, process resources and package apk artifacts, whereas lintDebug is used for analyzing code, for bugs, performance issues, security and styling deviations. Those two cases differ I think you should decide on which one to keep (build and package or debug variant analysis).
There was a problem hiding this comment.
Check out this comment:
ohs-player-libraryuses theandroidKotlinMultiplatformLibraryplugin, which doesn't registerlintDebug-assembleAndroidMainis the closest "compile + verify the Android variant" task it exposes.ohs-player-reference-appuses classicandroidTargetand keepslintDebug.
ohs-player-library uses the com.android.kotlin.multiplatform.library plugin, which doesn't register a lintDebug task, so lint isn't available there. assembleAndroidMain is the closest task it exposes that compiles and verifies the Android variant. ohs-player-reference-app is still a classic com.android.application + androidTarget, which does have lintDebug.
The goal of this leg isn't strictly "build" or strictly "analyze" - it's "verify the Android variant of both modules is healthy using whatever each module's plugin exposes": a compile/package check on the library and lint analysis on the app.
This asymmetry resolves itself during the AGP9 migration. The app will be split into a thin com.android.application module (androidApp) on top of KMP library modules, at which point ohs-player-reference-app becomes a com.android.kotlin.multiplatform.library and loses lintDebug - same as the library today. It will change to
tasks: >-
:ohs-player-library:assembleAndroidMain
:ohs-player-reference-app:assembleAndroidMain # both KMP modules: compile + verify
:androidApp:lintDebug # lint on the one real Android app module
I'd lean towards leaving this leg as-is until the AGP9 migration is ready.
| # Same reason as the js leg: compileTestDevelopmentExecutableKotlinWasmJs | ||
| # exhausts 6g; needs more daemon heap, but only on this runner. |
There was a problem hiding this comment.
This can be inferred from the setting below, no need to add this comment, the alternative make it brief.
| # iOS targets can't be compiled on Linux. macos-latest billing is 10× so | ||
| # keep each leg focused on a single arch so the K/N compiler isn't | ||
| # fighting itself for the 14 GB runner. Debug link is sufficient to | ||
| # prove iOS code compiles and links; release link belongs in release.yml | ||
| # whenever iOS distribution is added. |
There was a problem hiding this comment.
Paraphrase this comment and make it brief.
| sourceCompatibility = JavaVersion.VERSION_11 | ||
| targetCompatibility = JavaVersion.VERSION_11 |
There was a problem hiding this comment.
Is it possible to upgrade this to at least Java 17. I'm okay doing that in a followup PR.
There was a problem hiding this comment.
We can leave this to the AGP9 migration PR. We can also agree on the runtime floor to support then.
| // WiX/MSI requires a strict MAJOR.MINOR.PATCH numeric version, so strip any | ||
| // Semantic Version pre-release suffix (e.g. -alpha.1) from VERSION_NAME for the desktop | ||
| // installers. Android's versionName keeps the suffix; this drift is intentional. | ||
| // Uses the providers API so the configuration cache tracks VERSION_NAME as a | ||
| // declared input. |
There was a problem hiding this comment.
Improve this comment and provide example how the new version name will be on the respective platforms with before and after (stripping) versions.
| // Same as in ohs-player-library: Compose UI tests in commonTest don't run | ||
| // cleanly on Android-host JVM (needs Robolectric + per-class @RunWith) nor in |
There was a problem hiding this comment.
I do not think we need Robolectric with Compose multiplatform to run the UI tests. Refer to Compose Multiplatform UI test. We should adopt that.
Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com>
Co-authored-by: Elly Kitoto <junkmailstoelly@gmail.com>
- Update comments
- Disable failing JS/Wasm/Android-host test tasks only on CI - Rename helpers for clarity: envOrAbsent -> nonBlankEnv, envOrFile -> envOrKeystore - Expand build-script comments - Update release.yml workflow_dispatch dry-run description
Fixes #43