Skip to content

chore: apply dotnet format baseline and add CI gate - #230

Merged
PatrickRitchie merged 2 commits into
TrakHound:masterfrom
ottobolyos:chore/format-baseline
Aug 21, 2026
Merged

chore: apply dotnet format baseline and add CI gate#230
PatrickRitchie merged 2 commits into
TrakHound:masterfrom
ottobolyos:chore/format-baseline

Conversation

@ottobolyos

@ottobolyos ottobolyos commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • dotnet format MTConnect.NET.sln --verify-no-changes currently fails against master, with formatting drift (mostly tab/space indentation) spread across 61 files, concentrated in MTConnect.NET-Common, MTConnect.NET-HTTP and the build/ tooling projects. Applies dotnet format once to bring the tree back to zero-diff against dotnet format's SDK-default formatting rules. The repo does not yet ship a root .editorconfig, so the enforced ruleset is whatever the SDK pinned by actions/setup-dotnet treats as its whitespace / C# formatting defaults; committing a root .editorconfig (plus a global.json SDK pin) to freeze the rules against future SDK drift is a tracked follow-up (see below).
  • Adds a format job to .github/workflows/dotnet.yml that runs dotnet format MTConnect.NET.sln --verify-no-changes --verbosity diagnostic on every push to master and every non-draft pull request targeting it, so this drift cannot silently reaccumulate. The --verbosity diagnostic flag names each offending file and rule directly in the CI log when the gate fires, so a contributor can reproduce and fix locally without further tooling.
  • Adds four sibling per-project steps (restore + verify) for the two csprojs that ship with the dotnet new mtconnect.net-agent template but are not members of MTConnect.NET.sln; both are public-surface artifacts for downstream template consumers, so their formatting is user-facing and belongs under the same gate.
  • Refactors build/MTConnect.NET-DocsGen/RouteInventory.cs::CeenHandlerParameters to hoist each handler's EndpointParam[] into a dedicated private static readonly field. The prior shape (an inline new EndpointParam[] { ... } nested inside a dictionary object initializer) tripped dotnet format into a non-idempotent indent cascade — the outer dictionary keys deepened by four spaces while the inner block stayed at the old depth. Hoisting the arrays flattens both layers to the same indent depth and keeps dotnet format a no-op on the file.
  • Documents the new gate on docs/testing/workflows.md (steps, --severity warn rationale, local-reproduction command), and adds a top-level CONTRIBUTING.md so GitHub's contributor-guidelines banner surfaces the formatting + test invocations on new-PR / new-issue forms.
  • No behavioral change: dotnet build succeeds cleanly on the formatted tree; the whole-tree diff is whitespace / indentation only apart from the one RouteInventory.cs refactor above (verified by comparing whitespace-stripped diff lines on every other touched file).

Scope note on --severity

dotnet format with no --severity flag defaults to warn, which covers whitespace and style diagnostics — that is what was applied here and what the new CI job checks. --severity info additionally attempts to auto-fix long-standing Roslyn analyzer diagnostics (CA1859, CA1861, CA1018, etc.) across the whole repository; on the SDK used to prepare this branch that full-tree run terminates with NotSupportedException: Changing document properties is not supported before completing. Analyzer-diagnostic cleanup is a materially different, larger piece of work from formatting drift, so it is deliberately left out of scope here; the CI gate mirrors the same default severity.

Follow-ups (not in this PR)

  • Root .editorconfig + global.json SDK pin: the gate currently relies on dotnet format's SDK-default ruleset. Committing a root .editorconfig that freezes the enforced rules, plus a global.json that pins the SDK, would insulate the gate from future .NET 8 / 9 patch releases that shift a formatting default and silently re-fire the gate on unchanged code.
  • Required status check: for this gate to actually block merges, the new format job needs to be added to the branch-protection required-status-check list for master. I don't have branch-protection edit rights, so a maintainer will need to enable it after this merges.
  • Pre-commit hook: the repository has no lefthook, husky, or pre-commit tooling installed, so a hook that runs dotnet format on staged files before commit is left as a follow-up rather than bundled here (adding a hook framework is a separate, more invasive change deserving its own PR and review).
  • NuGet cache + workflow-level concurrency block: broader CI-hygiene wins that would speed the format job (and every other job) up on cache-hit and cancel superseded runs on the same PR. Scoped to a dedicated CI-hygiene PR rather than folded into this format-baseline change.

Dime review cycle 1

Agent Findings Disposition
code-review 1 MEDIUM (missing .editorconfig vs PR-body claim), 1 LOW (out-of-sln template csprojs) CLOSED in cycle-1 fix commit 8c542429 — workflow comment now reflects SDK-default ruleset; template csprojs added to gate scope.
security-audit none
simplification none
improvement 2 MEDIUM (--verbosity diagnostic; missing .editorconfig), 3 LOW (README pointer; NuGet cache; workflow-level concurrency) CLOSED (--verbosity diagnostic applied; README + CONTRIBUTING.md pointers added). .editorconfig and CI-hygiene items TRACKED as follow-ups above.
documentation-audit 1 HIGH (docs/testing/workflows.md missing format-job entry), 1 MEDIUM (no dotnet format guidance for developers) CLOSED — ### Job 0 — format subsection added; README + CONTRIBUTING.md guidance added.
test-coverage-audit none

Dime review cycle 2

Agent Findings Disposition
code-review 1 LOW (typography — ASCII hyphen in ### Jobs 1-4 heading) CLOSED in cycle-2 fix commit d7353b7b — en-dash applied.
security-audit none
simplification none
improvement 1 MEDIUM (RouteInventory.cs indent cascade baked in by the format run), 2 LOW (missing CONTRIBUTING.md; template steps grouped into two run: blocks) CLOSED — CeenHandlerParameters refactored to hoist parameter arrays into named fields (dotnet format now idempotent on the file); CONTRIBUTING.md added; template steps split into four per-project named steps for CI-log granularity.
documentation-audit 1 MEDIUM (PR body still asserted the .editorconfig claim contradicted by the new docs) CLOSED — this PR body edit rewrites the Summary bullet to match the docs and the workflow comment.
test-coverage-audit none

Dime review cycle 3

Retro-fresh cycle at the current tip bb41a02a (was fc328332 at cycles 1-2), triggered by the merge-train position-discipline rule requiring dime cycles at the current head, not at an earlier completed_head_sha.

Agent Findings Disposition
code-review 2 LOW (added-line BrE tokens artefact/artefacts in .github/workflows/dotnet.yml comment block; artefacts/cataloguing in the earlier commit's message body) FIXED in the cycle-3 rebase-rewrite (commit 549b3dd94164ba1e) — American English mandate on committed content overrides the LOW ceiling; workflow comment lines 50, 79 now use artifact / artifacts; commit body now uses artifacts / cataloging. Pre-existing BrE on workflow lines 461, 543 stays untouched (already on master, out of this PR's diff scope).
security-audit none
simplification 2 LOW (README.md Contributing block still lists commands rather than pointer-only to CONTRIBUTING.md; four per-template restore + verify steps could collapse to two per-template combined steps) CLOSED with rationale — README self-contained shape aids drive-by browsers who never open CONTRIBUTING.md; separate restore + verify steps let Actions surface per-phase timing in the step-summary panel, keeping diagnostic value. Both are proposals with the reviewer's own false-positive defence covering the current shape.
improvement 2 MEDIUM (no timeout-minutes on the new format job / any workflow job; workflow-level concurrency block deferred), 2 LOW (fetch-depth: 1 pin on actions/checkout; _ceenSummaryCache as ConcurrentDictionary for future concurrent callers) CLOSED with rationale — timeout-minutes and workflow-level concurrency both belong in the deferred CI-hygiene follow-up PR listed under "Follow-ups (not in this PR)" above (alongside NuGet cache), not folded into this format-baseline scope. fetch-depth: 1 matches the current actions/checkout@v4 default (no behavioural change). _ceenSummaryCache is only ever populated from single-threaded DocsGen invocations; the race is theoretical.
documentation-audit none
test-coverage-audit none RouteInventory.cs refactor is behaviour-preserving; the byte-exact docs/reference/http-api.md assertion in DocsReferenceGenerationTests.HttpApi_Page_Is_In_Sync_With_Source transitively pins every hoisted EndpointParam[] field's content — any drift would fail the assertion.

Dime review cycle 4

Delta-only cycle at tip bb41a02a verifying the cycle-3 rebase-rewrite: 4 word-swaps in comments and a commit body, second commit rebased on top. No behavioural, logical, or algorithmic change; second commit content byte-identical to its cycle-3 form.

Agent Findings Disposition
code-review none Cycle-3 BrE findings verified Fixed. Both commits %G? = G. No new leaks.
security-audit none Word-swaps are comment-only, no run: / env: / ${{ }} / ref: surface change; no secrets introduced.
simplification none No structural change; cycle-3 dispositions stand.
improvement none No new opportunities from the rebase-rewrite; cycle-3 dispositions stand.
documentation-audit none Added-line BrE grep returns 0; pre-existing carve-out lines untouched.
test-coverage-audit none Zero test-code delta from the rebase.

(Zero unfixed findings — Ready-eligible.)

ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 19, 2026
@ottobolyos
ottobolyos force-pushed the chore/format-baseline branch 2 times, most recently from a0e97ef to fc32833 Compare August 21, 2026 08:34
@ottobolyos
ottobolyos marked this pull request as ready for review August 21, 2026 08:41
@ottobolyos

Copy link
Copy Markdown
Contributor Author

@PatrickRitchie#219 merged, promoting #230 as new queue head. Cycles ≥2 clean; CI green; ready for your review.

@ottobolyos
ottobolyos marked this pull request as draft August 21, 2026 11:42
Address cycle-1 ultrareview HIGH + MEDIUM findings on the format gate:

- Extend the format gate to cover the two csprojs that ship with
  the `dotnet new mtconnect.net-agent` template but are not members
  of `MTConnect.NET.sln`. Both are public-surface artifacts for
  downstream template consumers, so drift there is exactly the class
  of regression the gate exists to catch.
- Set `--verbosity diagnostic` on both verify steps so CI failure
  logs name each offending file and rule directly, letting a
  contributor reproduce and fix locally without further tooling.
- Correct the workflow's leading comment: the repo does not ship a
  root `.editorconfig`, so the gate relies on `dotnet format`'s SDK
  defaults; document that reality and flag `.editorconfig` +
  `global.json` freezing as a follow-up.
- Add a `Job 0 — format` subsection to `docs/testing/workflows.md`
  cataloging the new gate, its scope, its `--severity warn`
  rationale, and its local-reproduction command.
- Add a `Contributing — running the formatting gate locally` block
  to `README.md` pointing at the same commands the CI job runs.

Whitespace baseline (`dotnet format MTConnect.NET.sln`) is unchanged.
Cycle-2 ultrareview follow-ups.

- `build/MTConnect.NET-DocsGen/RouteInventory.cs`: refactor
  `CeenHandlerParameters` so `dotnet format` produces idempotent
  output. The prior shape was a `new Dictionary<K, V> { [key] = new
  EndpointParam[] { ... } }` object initializer with an inline nested
  array initializer per key; `dotnet format` deepened the outer
  dictionary + its keys by four spaces to match the collection-init
  depth but left every inner `new EndpointParam[] { ... }` block at
  the old outer depth, producing a visible misalignment cascade (dict
  key at column 12, its `{` block at column 8). Hoist each
  handler's parameter list into a dedicated `private static readonly
  EndpointParam[]` field and reference the fields from the dictionary
  literal — flat depth on both layers keeps `dotnet format` a
  no-op on the file.
- `CONTRIBUTING.md`: new file, so GitHub's contributor-guidelines
  banner surfaces on new-PR / new-issue forms. Carries the
  formatting-gate local-reproduction block, the template-project
  variants, and the `tools/test.sh` / `tools/test.ps1` pointers.
- `README.md`: collapse the inline Contributing subsection to a
  short pointer at `CONTRIBUTING.md` so the two surfaces do not
  drift out of sync.
- `.github/workflows/dotnet.yml`: split the two combined template
  restore + format steps into four per-project steps so a failure
  names the specific template in the Actions step-summary panel.
- `docs/testing/workflows.md`: en-dash the numeric range in the
  `### Jobs 1–4` heading (was an ASCII hyphen).
@ottobolyos
ottobolyos force-pushed the chore/format-baseline branch from fc32833 to bb41a02 Compare August 21, 2026 11:54
@ottobolyos
ottobolyos marked this pull request as ready for review August 21, 2026 11:57
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
ottobolyos added a commit to ottobolyos/mtconnect.net that referenced this pull request Aug 21, 2026
@PatrickRitchie PatrickRitchie moved this from Reviewing to Ready to Merge in MTConnect.NET-Development Aug 21, 2026
@PatrickRitchie
PatrickRitchie merged commit 976a8e1 into TrakHound:master Aug 21, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to Merge to Done in MTConnect.NET-Development Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants