mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(scripts/check_emdash.sh): skip emdash check when no diff base is available (#26489)
## Problem `scripts/check_emdash.sh` is a diff gate for pull requests: it resolves the merge-base against the target branch and only inspects added lines. When it cannot resolve a base ref, it fell back to scanning **every tracked file**. Push builds on release branches hit exactly this case: the `lint` job checks out with `fetch-depth: 1`, so `origin/main` is absent, and `GITHUB_BASE_REF` is only set for `pull_request` events. With no base ref, the whole-tree scan flags the many pre-existing emdash/endash characters already in the repo and fails `make lint` (`lint/emdash`), even though the build introduced none of them. Observed on `release/2.34` CI (run [27704528068](https://github.com/coder/coder/actions/runs/27704528068/job/81949529546)). ## Fix When no base ref can be determined (i.e. outside a pull request), skip the check instead of scanning the entire tree. A full scan remains available on demand via `scripts/check_emdash.sh --all`. ## Testing - **No base ref** (release-push simulation, no `GITHUB_BASE_REF`, no `origin/main`): old script scans all files and fails on a pre-existing emdash; new script skips and exits 0. - **PR path** (diff vs merge-base): `OK: no emdash or endash characters found.` - **`--all`**: still scans the full tree (flags pre-existing characters as before). - `shellcheck` and `shfmt` clean. ## Backports Backport PRs target `release/2.33` and `release/2.34` (same bug, older script variant). `release/2.29` and `release/2.32` do **not** contain `scripts/check_emdash.sh`, so there is nothing to backport there. <details> <summary>Decision log</summary> Considered alternatives to the skip: 1. **Compare against `github.event.before`** on push events. Rejected: the before-SHA is frequently unreachable in a `fetch-depth: 1` clone, and wiring it in requires per-workflow env changes that complicate backports. 2. **Fetch `origin/main` / deepen history** in the release lint job. Rejected for the same backport-surface reason and because it only masks the design intent. The check exists to stop *new* emdashes from landing via PRs; that gate already ran on the originating PRs. On non-PR builds there is no meaningful diff base, so skipping is correct and self-contained in the script (clean to backport). The explicit `--all` mode is preserved for intentional full-tree audits. </details> --- Generated by Coder Agents on behalf of @f0ssel.
This commit is contained in:
+12
-5
@@ -175,12 +175,19 @@ else
|
||||
exit 1
|
||||
}
|
||||
if [[ -z "$base" ]]; then
|
||||
echo "WARNING: no base ref found, scanning all tracked files." >&2
|
||||
scan_all_files
|
||||
else
|
||||
found=0
|
||||
scan_diff "$base"
|
||||
# No base ref is available outside of pull requests, for
|
||||
# example push builds on release branches with a shallow clone
|
||||
# where neither GITHUB_BASE_REF nor origin/main is present.
|
||||
# The diff check has nothing to compare against, and scanning
|
||||
# every tracked file would flag pre-existing characters that
|
||||
# are unrelated to the change under test. Skip instead; pass
|
||||
# --all to force a full-tree scan.
|
||||
echo "OK: no base ref found (not a pull request); skipping emdash check."
|
||||
echo " Pass --all to scan every tracked file."
|
||||
exit 0
|
||||
fi
|
||||
found=0
|
||||
scan_diff "$base"
|
||||
fi
|
||||
|
||||
if [[ "$found" -ne 0 ]]; then
|
||||
|
||||
Reference in New Issue
Block a user