Skip to content

ci(build): enforce dotnet format so .editorconfig stops being aspirational (#301) - #307

Merged
phmatray merged 15 commits into
devfrom
feat/301-the-editorconfig-s-style-rules-are-enfor
Aug 13, 2026
Merged

ci(build): enforce dotnet format so .editorconfig stops being aspirational (#301)#307
phmatray merged 15 commits into
devfrom
feat/301-the-editorconfig-s-style-rules-are-enfor

Conversation

@phmatray

@phmatray phmatray commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Implements #301.

Closes #301.

Executing the implementation plan task-by-task; the checklist below — and the plan on the issue — are
ticked as each task lands. Opened as a draft — will be marked ready after the final task and a
code-review pass.

Plan

  • Task 1: A Format target that fails on the current tree
  • Task 2: The mechanical whitespace pass
  • Task 3: The code-style pass, with the shape-changing fixes read individually
  • Task 4: Make the gate binding, and correct the docs

⚠️ Merge timing

#301's Global Constraints say to land this when few PRs are open — the formatting pass touches
~200 files and will conflict with every in-flight branch. That constraint governs merging, not
building, so this PR gets built now and the timing decision stays with whoever merges it. At the time
of writing there were 6 active branches; consider landing those first, then re-syncing this one.

The conflicts are trivial to resolve (take either side, re-run dotnet format) but there will be a
lot of them.


What the implementation turned up

Three things the plan did not anticipate, all now documented in CLAUDE.md and the repo profile so
the next person does not rediscover them:

1. ⚠️ dotnet format writes merge-conflict markers into multi-targeted source files. All three
shipping projects are net8.0;net10.0, and a fix applied once per TFM can land as a literal
<<<<<<< TODO: Unmerged change from project 'FormCraft(net10.0)' block inside the .cs file
code that does not compile. Hit twice here (FieldRendererBase.cs, then two Fluent files). Verify
mode never writes, so the CI gate itself is unaffected; it only bites when applying. Mitigations
now recorded: check grep -rl '<<<<<<< TODO' --include='*.cs' . after any apply run, and scope with
--include <file> when a whole-solution run misbehaves.

2. One dotnet format run does not always reach a fixpoint. After formatting a field-keyword
property, verify still reported three WHITESPACE diagnostics on it; a second run cleared them. So
"I formatted and the gate still fails" is not necessarily a gate bug.

3. IDE0032 now rewrites private backing fields into C# 14's field keyword — and silently drops
the XML docs that lived on the field.
On MudBlazorFileUploadComponentBase.RequiredDescriptionId
that deleted the <remarks> explaining a real correctness invariant (#262 / CLAUDE.md: the hint id
must be unique per rendered instance, because item fields render one hint per row). The rewrite is
value-identical, so no test could have caught the loss. The explanation has been restored onto the
property, and this is called out in CLAUDE.md as a hunk type to read rather than skim.

Sync with dev

Merged origin/dev (3 commits, including #291's 5,600-line Fluent UI parity work). Git merged it
cleanly with no conflicts — but the merged code was written before this gate existed and brought
21 fresh violations, so it needed its own formatting pass (style: format the Fluent UI code merged from dev…). That is the expected steady state for this branch until it lands: every re-sync will
need one.

Also verified while there: #291 correctly flipped ValidateCollections="true" on the Fluent
container when it implemented collection rendering, which is the trap flagged on #278.

Follow-ups

  • Make the Format target refuse a tree containing <<<<<<< TODO markers. Finding 1 above is a
    silent corruption that currently only a human notices; a two-line guard in the target would turn it
    into a failed build. Out of scope here because the gate's job this PR is to verify formatting, not
    to police the formatter.
  • Revisit EnforceCodeStyleInBuild=true once this gate has held for a few releases. The .editorconfig's style rules are enforced nowhere, so 574 violations have accumulated #301 rejected
    it as the entry point (a hard build break until the last violation is fixed, plus an analyzer pass
    on every incremental build). With the tree already clean, that objection is much weaker — it would
    then cost one property and no diff.

⚠️ The automated code-review pass did not complete — read this before approving

implement-issue's Step 7 calls for a code-review pass over the branch. It was attempted three
times and never returned findings
, so this PR has not had one:

  1. First run destroyed uncommitted work — it does git checkout <ref> -- ., which reverted a
    formatting pass mid-flight (the hazard the skill itself documents). Stopped it, re-did the work,
    committed before retrying.
  2. Second run wedged: no output for ~9 minutes and no reply to a direct status request.
  3. Third run, scoped to just the five hand-written files, also failed to return.

Most likely environmental — a dozen agent sessions were running against this repo concurrently.

What stands in for it: a manual review of every non-mechanical hunk, which is where the three
findings above came from (the corruption, the deleted <remarks>, the non-convergence). The
formatting churn is tool output that ./build.cmd Format verifies in CI. But a second opinion on the
hand-written delta — build/Build.cs, ci.yml, FieldRendererBase.cs, FieldRendererService.cs,
MudBlazorFileUploadComponentBase.cs, DemoComponentBase.cs — is genuinely missing, so please give
those a human read rather than assuming a bot already did.

Verification at the ready-flip

@phmatray
phmatray marked this pull request as ready for review August 13, 2026 12:59
@phmatray

Copy link
Copy Markdown
Owner Author

Merge held — dev is red, and not because of this PR

A /merge-pr run stopped short of merging. dev's own CI is failing on its tip (65944e5):

Test  Failed  // Projects that did not complete: FormCraft.ForMudBlazor.UnitTests

Reproduced locally on a merge of origin/dev into this branch — one failing test, and it belongs to
neither side of that merge on its own:

FormCraft.ForMudBlazor.UnitTests.Fields.CollectionItemShapeGuardTests
  .No_Suite_Should_Re_Declare_A_Collection_Item_Shape_The_Fixture_Provides

  offenders should be empty but had 2 items:
    FieldConfigurationRefreshTests.EqualityItemModel holds List<EqualityItem>, whose shape (String)
      is already a CollectionItemFixture item type
    FieldConfigurationRefreshTests.EqualityItemModel is a collection root whose composition (String)
      is already modelled by CollectionItemFixture.NamedOrderModel or OrderModel

Cause: two PRs that never saw each other.

Each was green against the dev it branched from; together they are red. Nothing in #307 touches
either file.

Why this PR was not merged anyway. Merging into a red base would bury this PR's own signal and
make a repo-wide formatting change look like the thing that broke the build. The Format gate this
PR adds runs before Test in ci.yml, so a reader seeing red on dev afterwards would reasonably
suspect the new gate first.

Where this branch stands: at 1df9f0b, synced through #328, working tree clean. Its last two
heads (86c3222, fe2f586) both passed CI — Run ./build.cmd Format: success followed by
Run ./build.cmd Test: success. It currently reads DIRTY only because dev has since taken #306
and #331.

To land it: fix dev first (the guard's own message names the remedy — add EqualityItemModel
to FindOffenders' allowlist with a reason, or switch that suite to the shared fixture), then re-run
/merge-pr #307. The re-sync will need one more formatting pass for whatever landed meanwhile, which
is expected and is the cost #301 flagged for landing a repo-wide format change in a busy repo.

…fig-s-style-rules-are-enfor

# Conflicts:
#	FormCraft/Forms/Validation/DynamicFormValidator.cs
#	README.md
@phmatray
phmatray merged commit 8d23d45 into dev Aug 13, 2026
2 checks passed
@phmatray
phmatray deleted the feat/301-the-editorconfig-s-style-rules-are-enfor branch August 13, 2026 13:36
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.

The .editorconfig's style rules are enforced nowhere, so 574 violations have accumulated

1 participant