Support ZIP-packaged Linux release assets#20
Conversation
Oleg-Melnik
left a comment
Oleg-Melnik
left a comment
There was a problem hiding this comment.
Reviewed the ZIP-vs-bare Linux asset selection. Core logic is sound: I confirmed embed-code-go currently publishes only v1.2.3 and v1.2.4, both shipping the bare embed-code-linux asset, and the default version (v1.2.4) stays in the bare set — so behavior is correct today. Two inline notes below (one design/diagnostics, one test nit); no blocking defects.
| /** | ||
| * All releases published before Linux ZIP packaging was introduced. | ||
| */ | ||
| private val bareLinuxAssetReleases = setOf("v1.2.3", "v1.2.4") |
There was a problem hiding this comment.
Should fix (design / diagnostics): this allowlist inverts the trust boundary — every release not listed, including any future embed-code-go release, is assumed to ship embed-code-linux.zip. That is correct today (v1.2.3/v1.2.4 are the only releases and both ship the bare asset), but it hard-codes the assumption that every release after v1.2.4 packages Linux as a ZIP.
If a future release ships the bare asset instead, or the ZIP switch slips past v1.2.5, Linux installs fail with a plain HTTP 404 ... embed-code-linux.zip. addReleaseTagMigrationHint only covers a missing v prefix, so nothing points the user at the packaging change.
Consider (a) recording the cross-repo constraint — new embed-code-go releases must package Linux as .zip — in PROJECT.md's compatibility/trust notes or a comment here, and/or (b) extending the download-failure hint to mention the bare↔ZIP fallback.
|
|
||
| @Test | ||
| fun `select bare Linux asset for releases published before ZIP packaging`() { | ||
| listOf("v1.2.3", "v1.2.4").forEach { releaseTag -> |
There was a problem hiding this comment.
Nit: the forEach asserts without identifying the tag, so a regression for one release reports a bare equality failure with no indication of which tag failed. Passing the tag as the assertion message (assertEquals(expected, actual, releaseTag)) makes the failure self-describing. Also note this list literally duplicates the production bareLinuxAssetReleases set — acceptable, just kept in sync manually.
Oleg-Melnik
left a comment
Oleg-Melnik
left a comment
There was a problem hiding this comment.
@Vladyslav-Kuksiuk LGTM with comments to address.
2 participants
Select
embed-code-linux.zipfor releases after v1.2.4, while preserving bare Linux asset support for existing releases.