Skip to content

Make unit relation ordering culture-invariant#1688

Merged
angularsen merged 1 commit into
masterfrom
agl/unit-ordering
Jul 22, 2026
Merged

Make unit relation ordering culture-invariant#1688
angularsen merged 1 commit into
masterfrom
agl/unit-ordering

Conversation

@angularsen

@angularsen angularsen commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Extracted from #1544 because deterministic code generation is independent of the QuantityValue feature and improves the existing generator on its own. Moving it out reduces the size and conflict surface of the original PR.

Changes:

  • Deserialize relation definitions without relying on SortedSet's current-culture comparer.
  • Normalize relations with an ordinal, case-insensitive immutable sorted set before parsing and rewriting the file.

Tests:

  • No test cases changed; verified by running the full code generator and confirming it produced no unrelated generated-file changes.

Extracted from #1544 (#1544) because deterministic code generation is independent of the QuantityValue feature and improves the existing generator on its own. Moving it out reduces the size and conflict surface of the original PR.

Changes:
- Deserialize relation definitions without relying on SortedSet's current-culture comparer.
- Normalize relations with an ordinal, case-insensitive immutable sorted set before parsing and rewriting the file.

Tests:
- No test cases changed; verified by running the full code generator and confirming it produced no unrelated generated-file changes.
@angularsen angularsen changed the title Make unit relation ordering culture-invariant PR1544 split - Make unit relation ordering culture-invariant Jul 22, 2026
@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review

Small, well-scoped change to CodeGen/Generators/QuantityRelationsParser.cs — no changes to the public UnitsNet API or generated code, so there's nothing to flag on breaking changes, new quantities/units, or generated-code diffs.

The fix itself is correct and worth having. SortedSet<string> with no explicit comparer uses Comparer<string>.Default, which is culture-sensitive — so deserializing UnitRelations.json into a SortedSet<string> could produce a different sort order (and thus a different rewritten file) depending on the OS/thread culture the generator runs under. Switching to List<string> + ToImmutableSortedSet(StringComparer.OrdinalIgnoreCase) makes the ordering deterministic across environments, which is exactly the kind of thing that causes annoying "unrelated" diffs in CI vs. local runs.

Good choice using OrdinalIgnoreCase rather than plain Ordinal — it keeps the file's alphabetical look similar to before (avoids reshuffling everything to strict ASCII/ordinal order, e.g. all-uppercase-before-lowercase), while still being culture-invariant.

One subtlety worth a sentence in the PR description (low severity): ImmutableSortedSet de-duplicates by the comparer, not just orders by it. Two relation strings differing only by case would now silently collapse to one entry (the first added) instead of both surviving into parsedRelations. Previously, with the culture-sensitive default comparer, such strings would typically be treated as distinct and would have hit the duplicates/ambiguous checks in ParseAndApplyRelations with a helpful exception. In practice this is very unlikely to matter (relation strings encode PascalCase quantity/unit names, so a same-except-case collision would almost certainly be a typo), but it's a small trade-off: less noisy sorting vs. losing an error signal for that edge case.

Style-wise this matches the codebase conventions (using directives correctly ordered, nullable-friendly, adds the missing trailing newline). No test coverage was added, but that matches the existing pattern for CodeGen (no dedicated test project) and the PR description notes it was verified by running the full generator and diffing output — reasonable for a change of this size.

Nothing else stands out — no performance or security concerns given this only runs at code-gen/build time over a local JSON file.

@angularsen angularsen changed the title PR1544 split - Make unit relation ordering culture-invariant Make unit relation ordering culture-invariant Jul 22, 2026
@angularsen
angularsen enabled auto-merge (squash) July 22, 2026 23:01
@angularsen
angularsen disabled auto-merge July 22, 2026 23:03
@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96%. Comparing base (7e8da49) to head (05cf5db).

Additional details and impacted files
@@          Coverage Diff           @@
##           master   #1688   +/-   ##
======================================
  Coverage      96%     96%           
======================================
  Files         456     456           
  Lines       30107   30107           
======================================
  Hits        29043   29043           
  Misses       1064    1064           
Flag Coverage Δ
net48 96% <ø> (ø)

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 merged commit a9c805b into master Jul 22, 2026
7 checks passed
@angularsen
angularsen deleted the agl/unit-ordering branch July 22, 2026 23:21
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