perf(469): format gate uses dotnet format whitespace --folder (~480s → ~0.5s)
Build ErsatzTV Image / decisions.md append-only (pull_request) Waiting to run
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 17s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m15s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Has been cancelled
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been cancelled
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Has been cancelled
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Has been cancelled
Build ErsatzTV Image / decisions.md append-only (pull_request) Waiting to run
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 6s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 17s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m15s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 1m28s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Has been cancelled
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been cancelled
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Has been cancelled
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Has been cancelled
The blocking `format` CI job and the Husky pre-commit hook verified changed .cs files with `dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include <files>`. `--include` only narrows *which* files are checked, never what gets loaded: the full recipe loaded the ~10-project MSBuild workspace and built a Roslyn compilation per project before checking a single line (~480s locally, whole-solution). Switch both to `dotnet format whitespace . --folder --verify-no-changes --include <files>`, which treats the tree as a plain folder of files, skips MSBuild/Roslyn entirely (~0.5s), and needs no `dotnet restore` (NuGet-cache + Restore steps removed). Coverage is unchanged: folder mode reads .editorconfig and enforces exactly the gate's purpose — whitespace + charset (BOM). Proven non-vacuous (error WHITESPACE on a trailing-space line, error CHARSET on a prepended BOM, exit 0 clean). The full gate never enforced the style/analyzer pass either — a warning-severity naming violation passes the full solution format (exit 0) — and the analyzers that must block (NU1904, S3981) are enforced at compile via WarningsAsErrors, not by this job. Docs: ci-cd.md Formatting section + the obsolete #406 memory note; decisions.md. fixes #469 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -885,16 +885,26 @@ jobs:
|
||||
echo "Generated API artifacts are in sync."
|
||||
|
||||
# Formatting-as-you-touch gate (ersatztv#311): verify the .cs files THIS PR changed conform to
|
||||
# .editorconfig (style + charset=utf-8, i.e. no UTF-8 BOM). Scoped to changed files so it enforces
|
||||
# "normalize a legacy file when you touch it" WITHOUT a big-bang reformat of the ~2500 pre-existing
|
||||
# BOM files. A PR that touches no .cs skips the expensive steps and passes trivially (always reports
|
||||
# a status, so it is safe as a required check).
|
||||
# .editorconfig whitespace + charset=utf-8 (i.e. no UTF-8 BOM). Scoped to changed files so it
|
||||
# enforces "normalize a legacy file when you touch it" WITHOUT a big-bang reformat of the ~2500
|
||||
# pre-existing BOM files. A PR that touches no .cs skips the check and passes trivially (always
|
||||
# reports a status, so it is safe as a required check).
|
||||
#
|
||||
# ersatztv#469: uses `dotnet format whitespace . --folder`, NOT the full `dotnet format <sln>`.
|
||||
# `--folder` treats the tree as a plain folder of files and skips the MSBuild/Roslyn workspace load
|
||||
# + per-project compilation that dominated the old recipe (~8 min locally on a whole-solution run) —
|
||||
# `--include` only ever narrowed *which* files were checked, never what got loaded. Folder mode
|
||||
# reads .editorconfig and still flags WHITESPACE (indent/EOL/trailing/final-newline) and CHARSET
|
||||
# (BOM) violations — exactly what this gate exists to catch — in ~0.5s with no `dotnet restore`.
|
||||
# What it drops is the style/analyzer pass (naming/`var`/qualification), which this gate never
|
||||
# meaningfully enforced: those .editorconfig rules are :suggestion/:none severity. Full rationale +
|
||||
# non-vacuity evidence: docs/ci-cd.md → Formatting; docs/decisions.md.
|
||||
format:
|
||||
name: Formatting (changed .cs conform to .editorconfig)
|
||||
# Was on the `small` lane (ersatztv#390) to dodge a ~29 min queue; reverted to `ubuntu-latest`
|
||||
# in ersatztv#406 — `dotnet format` needs the .NET SDK and real memory, so it does not belong
|
||||
# in a lane sized for seconds-long shell jobs. See the api-docs job above for the full
|
||||
# rationale; server-management#604 grew this lane so the queue it was dodging is gone.
|
||||
# Folder-mode whitespace is now a seconds-long, low-memory job (no Roslyn workspace, unlike the
|
||||
# 3.95 GiB full `dotnet format` measured in #406), so it no longer needs the memory headroom that
|
||||
# kept it on `ubuntu-latest`. Left here to avoid re-touching the lane/memory-cap accounting; a
|
||||
# move to a lighter lane is a server-management capacity call (#604).
|
||||
runs-on: ubuntu-latest
|
||||
container:
|
||||
image: 192.168.1.95:3000/timothy/ersatztv-ci:07048b8
|
||||
@@ -924,26 +934,14 @@ jobs:
|
||||
echo "No .cs change -> skipping format verify (job passes)."
|
||||
fi
|
||||
|
||||
- name: Cache NuGet packages
|
||||
if: steps.detect.outputs.cs_changed == 'true'
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ~/.nuget/packages
|
||||
key: nuget-${{ runner.os }}-${{ hashFiles('Directory.Packages.props', 'global.json') }}
|
||||
restore-keys: nuget-${{ runner.os }}-
|
||||
|
||||
- name: Restore
|
||||
if: steps.detect.outputs.cs_changed == 'true'
|
||||
run: dotnet restore
|
||||
|
||||
- name: Verify formatting of changed .cs files
|
||||
if: steps.detect.outputs.cs_changed == 'true'
|
||||
shell: bash
|
||||
run: |
|
||||
mapfile -t files < /tmp/changed-cs.txt
|
||||
echo "Verifying ${#files[@]} changed .cs file(s) against .editorconfig..."
|
||||
if ! dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include "${files[@]}"; then
|
||||
echo "::error::One or more .cs files this PR touches don't conform to .editorconfig (formatting or a UTF-8 BOM). Run 'dotnet format ErsatzTV.sln --include <files>' and commit the result in THIS PR — the fix-as-you-touch convention (docs/contributing.md §7; ersatztv#311). Legacy files you did NOT touch are unaffected."
|
||||
echo "Verifying ${#files[@]} changed .cs file(s) against .editorconfig (whitespace + charset)..."
|
||||
if ! dotnet format whitespace . --folder --verify-no-changes --include "${files[@]}"; then
|
||||
echo "::error::One or more .cs files this PR touches don't conform to .editorconfig (whitespace or a UTF-8 BOM). Run 'dotnet format whitespace . --folder --include <files>' (or the full 'dotnet format ErsatzTV.sln --include <files>') and commit the result in THIS PR — the fix-as-you-touch convention (docs/contributing.md §7; ersatztv#311). Legacy files you did NOT touch are unaffected."
|
||||
exit 1
|
||||
fi
|
||||
echo "All changed .cs files conform to .editorconfig."
|
||||
|
||||
+8
-6
@@ -11,15 +11,17 @@ if [ -n "$root_png" ]; then
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# dotnet format on staged .cs files (repo root). Scoped to the staged files so we
|
||||
# don't pay the full-tree cost; skip entirely when no .cs is staged (avoids the
|
||||
# ~20-40s sln load for web-only commits).
|
||||
# dotnet format on staged .cs files (repo root). Uses `whitespace . --folder` — same recipe as
|
||||
# the CI `format` job (ersatztv#469): folder mode checks .editorconfig whitespace + charset (BOM)
|
||||
# without the MSBuild/Roslyn workspace load, so it runs in ~0.5s instead of the old ~20-40s sln
|
||||
# load. Keeping this identical to CI avoids a local hook that blocks on rules CI no longer enforces.
|
||||
# Skip entirely when no .cs is staged (avoids any cost for web-only commits).
|
||||
cs_files=$(git diff --cached --name-only --diff-filter=ACM -- '*.cs')
|
||||
if [ -n "$cs_files" ]; then
|
||||
echo "husky - dotnet format (verify) on staged .cs files"
|
||||
echo "husky - dotnet format (whitespace verify) on staged .cs files"
|
||||
# shellcheck disable=SC2086
|
||||
dotnet format ErsatzTV.sln --verify-no-changes --include $cs_files || {
|
||||
echo "husky - dotnet format found issues in staged .cs files; run 'dotnet format ErsatzTV.sln --include <files>' to fix"
|
||||
dotnet format whitespace . --folder --verify-no-changes --include $cs_files || {
|
||||
echo "husky - dotnet format found whitespace/BOM issues in staged .cs files; run 'dotnet format whitespace . --folder --include <files>' to fix"
|
||||
exit 1
|
||||
}
|
||||
fi
|
||||
|
||||
+30
-7
@@ -145,9 +145,12 @@ invocation without touching each call site (MSBuild surfaces env vars as propert
|
||||
`UseSharedCompilation` is only defaulted to `true` when empty, so the env var wins).
|
||||
|
||||
**What this does and does not shrink.** It helps the jobs that *compile* — `test`, `migrations`,
|
||||
`api-docs` on an API-touching PR, and the in-Docker `build`. It does **not** help `format`: `dotnet
|
||||
format` loads Roslyn in-process via MSBuildWorkspace and never spawns `csc`, so its measured 3.95
|
||||
GiB is unaffected. Don't size the `small` lane expecting `format` to have shrunk — it hasn't.
|
||||
`api-docs` on an API-touching PR, and the in-Docker `build`. It never helped `format`: `dotnet
|
||||
format` loaded Roslyn in-process via MSBuildWorkspace and never spawned `csc`, so the compiler-server
|
||||
env vars left its measured 3.95 GiB untouched. (Moot since **ersatztv#469** switched `format` to
|
||||
`dotnet format whitespace . --folder`, which skips the MSBuild/Roslyn workspace entirely — the job is
|
||||
now a ~0.5 s, low-memory whitespace/BOM check with no Roslyn heap. See *Static analysis &
|
||||
formatting → Formatting* below.)
|
||||
|
||||
The same three are repeated in `dependency-scan.yml`; workflow `env:` does not cross workflow files.
|
||||
That one is lower-stakes (restore/list are MSBuild-driven, so it's lingering worker nodes rather
|
||||
@@ -674,10 +677,30 @@ Promoted rules are recorded here so the blocking subset stays intentional and re
|
||||
as a defensive default if server-rendered view code is ever reintroduced.
|
||||
|
||||
**Formatting** — the inherited tree still contains legacy UTF-8 BOM/whitespace debt, so the standing
|
||||
policy is **format as you touch**, not a mass rewrite (ersatztv#311). The Husky pre-commit hook runs
|
||||
`dotnet format --verify-no-changes` for staged C# files, and the blocking `format` CI job repeats that
|
||||
check for C# files changed by the PR. Untouched legacy files remain outside the gate; `.gitattributes`
|
||||
pins line endings. A one-time full-tree normalization remains a separate, unmade decision.
|
||||
policy is **format as you touch**, not a mass rewrite (ersatztv#311). The Husky pre-commit hook and
|
||||
the blocking `format` CI job both run `dotnet format whitespace . --folder --verify-no-changes
|
||||
--include <changed .cs>` — scoped to the files the commit/PR touches. Untouched legacy files remain
|
||||
outside the gate; `.gitattributes` pins line endings. A one-time full-tree normalization remains a
|
||||
separate, unmade decision.
|
||||
|
||||
*Why `whitespace . --folder`, not the full `dotnet format <sln>` (ersatztv#469)* — the gate only
|
||||
needs to enforce `.editorconfig` **whitespace** (indent/EOL/trailing/final-newline) and **charset**
|
||||
(no UTF-8 BOM). The old recipe (`dotnet format ErsatzTV.sln --no-restore --verify-no-changes
|
||||
--include`) loaded the entire ~10-project MSBuild workspace and built a Roslyn compilation per
|
||||
project *before* checking a single file — `--include` narrows *which* files are checked, never what
|
||||
gets loaded. Measured whole-solution `dotnet format` ran **~480s locally**; folder mode runs in
|
||||
**~0.5s** and needs no `dotnet restore` (the NuGet-cache + Restore steps were removed from the job).
|
||||
`--folder` treats the tree as a plain folder of files, skipping MSBuild/Roslyn entirely, and still
|
||||
reads `.editorconfig`. Verified **non-vacuous**: it exits non-zero on an injected trailing-whitespace
|
||||
line (`error WHITESPACE`) and on a prepended UTF-8 BOM (`error CHARSET`), and exits 0 on a clean file.
|
||||
**No coverage was lost**: the full `dotnet format` gate did **not** enforce the style/analyzer pass
|
||||
either — a probe injecting a `warning`-severity naming violation (`local_constants` not `ALL_UPPER`)
|
||||
passed the *full* solution format (exit 0), because the only `.editorconfig` rule above
|
||||
`:suggestion`/`:none` severity is that one naming rule and `dotnet format`'s default severity ignores
|
||||
it. The analyzers that **must** block (`NU1904`, `S3981`) are enforced at *compile time* via
|
||||
`WarningsAsErrors` in `Directory.Build.props`, not by this job. Devs fix a violation with `dotnet
|
||||
format whitespace . --folder --include <files>` (the full `dotnet format ErsatzTV.sln --include
|
||||
<files>` is a superset and also works).
|
||||
|
||||
## Migration integrity (EF Core, both providers)
|
||||
|
||||
|
||||
@@ -2014,3 +2014,33 @@ performance half.
|
||||
per client and the TTL collapses steady-state load, so at most a handful of exactly-simultaneous cold callers
|
||||
re-run — a once-per-30s edge, not the repeated per-request cost the issue targets. Recorded here so a later
|
||||
reviewer doesn't read the absence of a `SemaphoreSlim` as an oversight.
|
||||
|
||||
## 2026-07-19 — The `format` gate runs `dotnet format whitespace . --folder`, not the full solution format (#469)
|
||||
|
||||
The blocking `format` CI job (and the matching Husky pre-commit hook) verify changed `.cs` files with
|
||||
`dotnet format whitespace . --folder --verify-no-changes --include <files>` instead of the previous
|
||||
`dotnet format ErsatzTV.sln --no-restore --verify-no-changes --include <files>`.
|
||||
|
||||
- **Why.** `--include` narrows *which* files are checked, never what gets loaded. The full recipe loaded
|
||||
the whole ~10-project MSBuild workspace and built a Roslyn compilation per project before checking a
|
||||
single line — a fixed cost independent of how few files changed. Measured **~480s** for a whole-solution
|
||||
`dotnet format` locally (matching the issue's "7+ min"). `--folder` treats the tree as a plain folder of
|
||||
files and skips MSBuild/Roslyn entirely: **~0.5s**, and it needs no `dotnet restore`, so the job's
|
||||
NuGet-cache + Restore steps were deleted. It also drops the job's ~3.95 GiB Roslyn heap (the #406
|
||||
memory note about `format` not shrinking is now moot).
|
||||
- **Coverage is unchanged, not merely "good enough".** Folder mode reads `.editorconfig` and enforces
|
||||
exactly the two things this gate exists for — **whitespace** (indent/EOL/trailing/final-newline) and
|
||||
**charset** (no UTF-8 BOM). Proven non-vacuous: exits non-zero with `error WHITESPACE` on an injected
|
||||
trailing-whitespace line and `error CHARSET` on a prepended BOM; exits 0 on a clean file. What it drops
|
||||
is the style/analyzer pass — but the *full* gate never enforced that either: a probe injecting a
|
||||
`warning`-severity naming violation (`local_constants` not `ALL_UPPER`) **passed** the full solution
|
||||
format (exit 0), because the only `.editorconfig` rule above `:suggestion`/`:none` severity is that one
|
||||
naming rule and `dotnet format`'s default severity ignores it. The analyzers that must block (`NU1904`,
|
||||
`S3981`) are enforced at compile time via `WarningsAsErrors` in `Directory.Build.props`, never by this
|
||||
job.
|
||||
- **Fix command for a violation:** `dotnet format whitespace . --folder --include <files>`. The full
|
||||
`dotnet format ErsatzTV.sln --include <files>` is a superset (also applies style) and still works, so
|
||||
existing muscle memory and the #311 lore's `dotnet format --include` guidance are not broken.
|
||||
- **Lane left on `ubuntu-latest`.** The job is now seconds-long and low-memory, so it could move to a
|
||||
lighter lane, but that re-touches the per-lane memory-cap accounting (#406/#604) and is a
|
||||
server-management capacity call — deliberately out of scope here.
|
||||
|
||||
Reference in New Issue
Block a user