Skip to content

Improve UnitsNet.Modular onboarding - #1717

Merged
angularsen merged 1 commit into
masterfrom
agl-codex/modular-onboarding
Aug 1, 2026
Merged

Improve UnitsNet.Modular onboarding#1717
angularsen merged 1 commit into
masterfrom
agl-codex/modular-onboarding

Conversation

@angularsen

Copy link
Copy Markdown
Owner

Motivation

Make the first UnitsNet.Modular experience discoverable from IntelliSense, compiler diagnostics, documentation, and a package-based sample that behaves like a real consumer.

Changes

  • expand the authoring API XML documentation with complete module, unit-set, custom-spec, and profile examples
  • link every UNM diagnostic to the relevant README section and verify that coverage in a generator test
  • replace the quick start with two copy-pasteable files, explain generated ownership and relationships, and add troubleshooting guidance plus a scenario-oriented sample index
  • add an isolated NuGet getting-started sample that exactly matches the documented quick start
  • run both package-facing consumer scenarios in the separate UnitsNet.Modular workflow

Validation

  • dotnet test UnitsNet.Modular/UnitsNet.Modular.slnx --no-restore -m:1 -p:UnitsNetModularSampleUpdateLocalPackagesOnBuild=false (115 tests passed; runtime targets net8.0, net9.0, and net10.0)
  • pwsh UnitsNet.Modular/Samples/UnitsNet.Modular.GettingStarted.Sample/run.ps1
  • verified generated XML documentation contains the examples and links
  • verified all diagnostic anchors exist and all relative Markdown links resolve

@claude

claude Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review

Scope note: this PR touches only UnitsNet.Modular (docs, samples, CI, generator diagnostics, XML docs) — the design-probe subproject. No changes to Common/UnitDefinitions/, no new quantities/units, and no changes to the core UnitsNet/GeneratedCode/* used by the stable library. So the "new quantities/units" and "generated code" checklist items don't really apply here; feedback below is scoped accordingly.

Breaking changes

None. This is docs, samples, a new test, and additive helpLinkUri metadata on existing diagnostics. UnitsNetModularGenerator.cs diagnostic IDs/severities/messages are unchanged — only helpLinkUri is added, which is purely additive to DiagnosticDescriptor.

Generated code / generator changes

The 13 DiagnosticDescriptors in UnitsNetModularGenerator.cs each gain a helpLinkUri pointing at a README anchor (e.g. UNM001#select-quantities, UNM014/UNM013#module-declaration). I checked all the anchors referenced (#select-quantities, #filter-units, #quantity-definition-json, #register-a-definition, #add-quantity-relationships, #module-declaration, #arithmetic-and-relationships, #namespaces) against UnitsNet.Modular/README.md headings and they all resolve to real sections — good.

One gap: DiagnosticDocumentationTests.EveryDiagnosticLinksToRelevantDocumentation only asserts the URL prefix (Assert.StartsWith(".../UnitsNet.Modular#", ...)) and a hardcoded count of 13. It doesn't actually verify the anchor fragment corresponds to an existing README heading, so a future rename of a ##/### section (or a typo'd anchor on a new diagnostic) wouldn't be caught by CI — you'd have to rely on manual verification again, as the PR description notes. Might be worth having the test parse README.md headings and assert each helpLinkUri fragment matches one, so this stays enforced rather than manually re-verified each time. Not blocking, but flagging since the PR explicitly calls out wanting this "coverage."

The hardcoded Assert.Equal(13, ...) is a reasonable trip-wire to force a documentation update when a diagnostic is added, but it will need a one-line bump each time — minor friction, seems intentional.

Style and conventions

  • New sample project (UnitsNet.Modular.GettingStarted.Sample) mirrors the existing UnitsNet.Modular.NuGet.Sample csproj structure almost exactly (same LocalPackages.targets import, same RestoreSources/PackageReference pattern) — good consistency.
  • XML doc additions in AuthoringContracts.cs are thorough and follow the existing <summary>/<remarks>/<example>/<seealso> pattern already used elsewhere in the codebase.
  • File header comments (// Licensed under MIT No Attribution...) are present on new files, consistent with repo convention.

Code quality / potential issues

  • run.ps1 for the new sample closely duplicates UnitsNet.Modular.NuGet.Sample/run.ps1 (timestamped restore path, same -p: flags). Given there are now two near-identical scripts, a shared helper script could reduce duplication, though this is consistent with how the existing sample already does it, so it's a pre-existing pattern rather than something this PR introduces.
  • README quick start (UnitsNet.Modular/README.md) and the new GettingStarted.Sample are kept in sync manually (two files, copy-pasted snippets). There's no automated check that the README code blocks match the sample source verbatim, so they can silently drift on a future edit to either. Worth considering a small test that extracts the README's fenced code blocks and diffs them against the sample files, if drift is a concern long-term.

Test coverage

The new DiagnosticDocumentationTests is a reasonable addition (see gap noted above). No other production logic changed, so no additional test coverage is needed beyond that.

Security

None identified — this is docs/samples/test-only content plus additive diagnostic metadata (a URL string), no new attack surface.

Overall: solid, low-risk documentation/onboarding improvement. Main suggestion is tightening DiagnosticDocumentationTests to actually validate anchors against README headings rather than just the URL prefix, so the "linked and verified" guarantee is enforced by CI rather than manual review.

@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98%. Comparing base (af17f9b) to head (4209ea2).

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1717   +/-   ##
======================================
  Coverage      98%     98%           
======================================
  Files         514     514           
  Lines       24141   24141           
======================================
  Hits        23748   23748           
  Misses        393     393           
Flag Coverage Δ
net48 97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@angularsen
angularsen marked this pull request as ready for review August 1, 2026 19:59
@angularsen
angularsen merged commit 5aad11f into master Aug 1, 2026
9 checks passed
@angularsen
angularsen deleted the agl-codex/modular-onboarding branch August 1, 2026 20:56
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.

1 participant