chore(cli): deprecate event release mode on the mobile uploads - #92401
chore(cli): deprecate event release mode on the mobile uploads#92401ablaszkiewicz wants to merge 5 commits into
Conversation
proguard upload and dsym upload bind their symbol sets to the release they create. --release-mode and --no-release-bind are gone from both. A proguard map id is a hash of the mapping, and a dSYM symbol set is keyed on the Mach-O LC_UUID, so two releases collide only when they ship a byte-identical artifact. The dSYM path also lost release attribution for embedded targets, because one upload covers every target while it creates one release. --release-mode stays on the sourcemap and hermes commands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes. |
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 4 should fix, 0 consider. Published 4 findings (view the review). Resolved comments: 1 fixed, 1 declined, 2 left for you |
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
The proguard upload no longer has a release mode, but the comment on the release binding still described "both modes" and "Event mode" and ended in a truncated sentence. Replace it with an accurate comment about the binding. Generated-By: PostHog Desktop Task-Id: 749c89e8-6e0a-44af-a343-1652e26a485e
Removing --release-mode took its only two tests with it. The module kept a parse helper that nothing called, which CI rejects under -D warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
|
Reviews (1): Last reviewed commit: "chore(cli): drop the proguard upload tes..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
Removes the experimental “event release mode” controls from the mobile debug-symbol upload commands in the Rust CLI, standardizing proguard upload and dsym upload on binding uploaded symbol sets to the release they create.
Changes:
- Remove
--release-modefromproguard uploadand always setrelease_idon the uploaded mapping (when a release is created). - Remove
--no-release-bindfromdsym uploadand always setrelease_idon uploaded dSYM chunks. - Update shared conflict-flag help text and add a CLI changeset describing the behavior change.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cli/src/sourcemaps/args.rs | Updates help text for shared upload conflict flags. |
| cli/src/proguard/upload.rs | Drops --release-mode support and always attempts to bind proguard mappings to the created release. |
| cli/src/dsym/upload.rs | Drops --no-release-bind and always binds dSYM chunks to the created release. |
| cli/.sampo/changesets/remove-mobile-release-mode.md | Adds release notes for the CLI behavior/flag removal. |
Suppressed comments (1)
cli/src/sourcemaps/args.rs:145
- Similarly, the
--skip-on-conflicthelp text mentions “a sourcemap upload” and--release-mode=event, but this arg struct is reused by other commands that don’t support--release-mode. Rewording to describe the behavior without tying it to a specific subcommand/flag will avoid confusing CLI help across commands.
/// Skip symbol sets that already exist with different content instead of failing.
/// Existing symbol sets are left unchanged. A sourcemap upload ignores this with
/// `--release-mode=event`. [default: false]
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The proguard comment said the mapping binds to a release. A build without a release name or version resolves none, and --skip-release-on-fail can drop the binding too. UploadConflictArgs is flattened into six commands, and its help named sourcemap uploads. Three of those commands take no --release-mode, and hermes upload does and was not covered. The text now names the condition rather than a command. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
Reviewers asked for the mobile flag removal to stay backwards compatible: a released gradle plugin or upload-symbols.sh that still passes --release-mode or --no-release-bind must not fail to parse on a CLI upgrade. Both flags stay accepted and hidden, print a deprecation warning, and change nothing. proguard upload no longer reads POSTHOG_RELEASE_MODE, so the variable only steers the sourcemap and hermes commands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177eam86deHk8HZYMKcJtbD
ablaszkiewicz
changed the title
| /// Deprecated: the symbol sets always bind to the release the build creates. The flag stays | ||
| /// accepted so a released posthog-ios upload-symbols.sh that still passes it does not fail | ||
| /// the Xcode build with a parse error. | ||
| #[arg(long, default_value_t = false, hide = true)] |
There was a problem hiding this comment.
funny thing I only TIL'd, this hides it from --help but still shows if you type --no-release-bin -> "tip: a similar argument exists: '--no-release-bind'" 😂 (nothing actionable here, just sharing)
| if *no_release_bind { | ||
| tracing::warn!( | ||
| "--no-release-bind is deprecated and does nothing. The symbol sets are uploaded bound \ | ||
| to the release this build creates. Remove the flag." |
There was a problem hiding this comment.
the "remove the flag" might be misleading since it'd come from the gradle plugin / upload-symbols.sh, not really actionable, but more of a nit, not sure a better phrasing
Related PRs
Event mode is live today, and it is opt-in. These PRs settle where it stays and where it goes.
Deprecating the mobile knobs. The symbol id on these paths is already a content hash, so two releases collide only when they ship a byte-identical artifact. An ordinary release that changes code gets its own symbol set and never collides. The dSYM path also lost release attribution for embedded targets, because one upload covers every target while it creates one release. Review asked for deprecation instead of removal, so every knob stays accepted as a warned no-op.
Making it the default.
React Native. Scopes the mode to the Hermes upload and defaults it to event. That is the one path where two releases really do ship the same artifact.
Problem
LC_UUID.Changes
proguard upload --release-modeanddsym upload --no-release-bindbecome hidden no-ops. Both commands always bind their symbol sets to the release they create.proguard uploadno longer readsPOSTHOG_RELEASE_MODE. The variable keeps steering the sourcemap and hermes commands, so a globalsymbol-setopt-out for those cannot trigger warnings here.--release-modestays visible onsourcemap inject,sourcemap process,sourcemap upload,hermes inject,hermes cloneandhermes upload.--release-mode", because proguard now takes it invisibly.How did you test this code?
--help, and--release-modebinds no environment variable. They catch a future re-removal, which is the build break this PR prevents.--release-mode eventprinted the warning and kept the release binding.Automatic notifications
Docs update
None. Both knobs were experimental and undocumented.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with Claude Code (Opus 5); reshaped from removal to deprecation with Claude Code (Fable 5). Skills invoked:
/writing-pr-descriptions.Reviews here and on PostHog/posthog-android#747 and PostHog/posthog-ios#791 asked for warned no-ops instead of removal. #91823 stacks on this PR and flips the defaults for the commands that keep the flag.