Make mob.uninstall actually honour :ios_bundle_id, and stop the deploy error lying - #68
Merged
Merged
Conversation
…y error lying Both found by an adversarial review of a documentation PR — the docs claimed these behaved correctly, and checking the claims found that they did not. Two defects I shipped, in the two commits before this one. `mix mob.uninstall` resolved one bundle id for both platforms in the task and passed it down as `project_bundle_id`. `resolve_apps_for_device/3` then did `opts[:project_bundle_id] || default_bundle_id(device)`, so the `||` short-circuited and the per-platform clause — the entire point of the change — was dead code on every real invocation. On a device holding the app under a different `:ios_bundle_id`, uninstall removed nothing and reported success. The test could not see it: it passed `[]` for opts, which is not the shape the task sends. Resolving the id now happens where the device is known, the task passes only whether it is in a Mix project, and the tests use the task's own opts. One of them asserts the two shapes still agree, since that mismatch is what hid the bug rather than the logic itself. Separately, `mix mob.deploy`'s invalid-option error advised that a value beginning with `-` needs the equals form. That was true until `join_dashed_values/1` landed in the same commit as the advice and made the spaced form work. Telling someone their working spelling is wrong is a small version of the thing this whole run has been about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Strict option parsing turned `mix mob.deploy --help` into `Unrecognized or invalid option(s): --help` — the flag people try first, on the task they run most, telling them to go and read the help. `mix mob.mutate` shipped with the same hole a day later. `mix mob.uninstall` had already solved this: `MobDev.TaskHelp` exists for it, and its moduledoc explains why (Mix has `mix help <task>`, but people from outside the Elixir ecosystem expect `<command> --help` to work). Both tasks now use it. Found by an adversarial review of the documentation PR, which noticed that a README paragraph celebrating the new refusal did not mention that it also refused --help. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both found by an adversarial review of a documentation PR. The docs claimed these behaved correctly; checking the claims found that they did not. Two defects I shipped, in the two commits before this one.
mix mob.uninstallignored:ios_bundle_identirelyThe task resolved one bundle id for both platforms and passed it down as
project_bundle_id.resolve_apps_for_device/3then didopts[:project_bundle_id] || default_bundle_id(device)— the||short-circuits, so the per-platform clause that was the entire point of the change was dead code on every real invocation. On a device holding the app under a different:ios_bundle_id, uninstall removed nothing and reported success.The test could not see it: it passed
[]for opts, which is not the shape the task sends. Resolution now happens where the device is known, the task passes only whether it is in a Mix project, and the tests use the task's own opts. One asserts the two shapes still agree — that mismatch is what hid the bug, more than the logic did.The deploy error told you your working spelling was wrong
mix mob.deploy's invalid-option message advised that a dash-prefixed value needs the equals form. That was true untiljoin_dashed_values/1landed in the same commit as the advice and made the spaced form work. A small version of the thing this whole run has been about.2333 tests, credo and format clean.