Skip to content

Teach both installers the telemetry opt-out's new name - #7

Merged
mattpodwysocki merged 1 commit into
mainfrom
installer-telemetry-name
Sep 14, 2026
Merged

mattpodwysocki merged 1 commit into
mainfrom
installer-telemetry-name

Conversation

@mattpodwysocki

Copy link
Copy Markdown
Contributor

Ported from mapbox-cli-private#141, which can no longer merge there now that oss/ is a submodule (#132).

The gap

The binary's telemetry opt-out was renamed from DISABLE_TELEMETRY to MAPBOX_CLI_NO_TELEMETRY, and that break was documented honestly — the changelog says a DISABLE_TELEMETRY=1 "silently stops opting out". Both installers were left on the old name, so after that release neither variable silenced both halves:

Set mapbox binary install.sh / install.ps1
MAPBOX_CLI_NO_TELEMETRY=1 silent still reports
DISABLE_TELEMETRY=1 reports (documented) silent

The private repo's docs/distribution.md already claimed the fix:

MAPBOX_CLI_NO_TELEMETRY switches all of that off … Both installers carry their own copy of this logic, and both test suites pin it in both directions.

The first half was false, and the second pinned the wrong name. So this makes the documentation true rather than changing it.

The old name keeps working, in the installers only

Asymmetric on purpose. The binary's break was announced in a changelog someone upgrading can read. An installer is fetched and executed in one line — curl … | sh, irm … | iex — so its reader has no release notes in front of them, and breaking an opt-out is the one change that must not happen quietly. The comment in each file says exactly that, so the asymmetry reads as chosen rather than as an oversight in the other direction.

When both are set the new name wins, including when it declines: an explicit MAPBOX_CLI_NO_TELEMETRY=0 beats a stale DISABLE_TELEMETRY=1 left in an image from before the rename. Without that the old variable could never be retired.

Verified rather than reasoned about

I pulled each script's decision block out and ran it over the cases directly — nine in sh, eight in pwsh 7.6.6 — covering both names, both precedence directions, the six false spellings, an unknown spelling (which must send less), whitespace, and a set-but-empty new name as a deliberate clear. All pass.

Then added cases to both real harnesses, which pass here too:

MAPBOX_CLI_NO_TELEMETRY is honoured, and outranks the old name
  ok    exits 0 — the install is not what is being switched off
  ok    still names the installer
  ok    no platform rides behind it
  ok    and no source tag either
  ok    the new name wins when the two disagree
  ok    and wins in the opt-out direction too

Both suites green end to end in this repo: scripts/test-install.sh reports all cases passed, scripts/test-install.ps1 reports all good (2 pre-existing Windows-only skips, unrelated).

new_case_env / New-CaseEnv now clear the new variable too, for the same reason they already cleared the old one — a developer with it set in their own shell would otherwise turn every marker case into a failure that looks like the marker broke.

The original PR also edited docs/distribution.md, which lives in the private repo and stays there.

@mattpodwysocki
mattpodwysocki force-pushed the installer-telemetry-name branch 2 times, most recently from b56a3df to 831fff5 Compare September 14, 2026 17:55

@zmofei zmofei left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM — precedence logic and installer parity verified, tests cover both directions, CI green. Note: PR has a merge conflict with base, needs rebase before merging.

The binary's opt-out was renamed from `DISABLE_TELEMETRY` to
`MAPBOX_CLI_NO_TELEMETRY` and the break was documented honestly. Both
installers were left on the old name, so after that release neither variable
silenced both halves: the documented one stopped the CLI's markers but not the
installer's, and the old one did the reverse.

`docs/distribution.md` in the private repo already claimed the fix —
"`MAPBOX_CLI_NO_TELEMETRY` switches all of that off … Both installers carry
their own copy of this logic, and both test suites pin it in both directions."
The first half was false and the second pinned the wrong name. This makes that
documentation true rather than changing it.

**The old name keeps working here, and only here.** The binary's break was
announced in a changelog someone upgrading can read. An installer is fetched
and executed in one line — `curl … | sh`, `irm … | iex` — so its reader has
no release notes in front of them, and breaking an opt-out is the one change
that must not happen quietly. Asymmetric on purpose, and the comment in each
file says so.

When both are set the new name wins, including when it declines: an explicit
`MAPBOX_CLI_NO_TELEMETRY=0` beats a stale `DISABLE_TELEMETRY=1` left in an
image from before the rename. Without that the old variable could never be
retired.

Verified rather than reasoned about: each script's decision block was run over
the cases directly — nine in `sh`, eight in `pwsh` 7.6.6 — covering both
names, both precedence directions, the six false spellings, an unknown
spelling, whitespace, and a set-but-empty new name as a deliberate clear. Both
real harnesses then got cases asserting the new name opts out, wins when the
two disagree, and wins in the opt-out direction too. `test-install.sh` reports
all cases passed; `test-install.ps1` reports all good.

Ported from mapbox/mapbox-cli-private#141, which cannot merge there now that
`oss/` is a submodule (mapbox/mapbox-cli-private#132). That PR also edited
`docs/distribution.md`, which lives in the private repo and stays there.
@mattpodwysocki
mattpodwysocki force-pushed the installer-telemetry-name branch from 831fff5 to eb5e140 Compare September 14, 2026 18:05
@mattpodwysocki
mattpodwysocki merged commit e26f618 into main Sep 14, 2026
7 of 8 checks passed
@mattpodwysocki
mattpodwysocki deleted the installer-telemetry-name branch September 14, 2026 18: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.

2 participants