Support partial unit systems#1690
Conversation
Extracted from #1544 (#1544) because representing a unit system for only the dimensions an application uses is useful independently of QuantityValue. Moving it out gives the API change focused review and reduces the original PR's scope. Changes: - Allow UnitSystem to be constructed from any BaseUnits value defining at least one dimension. - Continue rejecting null and fully undefined base-unit sets. - Clarify the constructor documentation and exception message. Tests: - Change the seven previously rejected one-dimension-missing cases to assert successful construction. - Add a single-dimension UnitSystem case. - Add a dedicated assertion that BaseUnits.Undefined remains invalid. - Verify all UnitSystemTests on net10.0 and net48 (16 tests per target).
Review of #1690 — Support partial unit systemsSmall, focused, well-tested change. Summary of what I checked: Breaking changesNone. The change only relaxes validation in Style & conventionsMatches existing conventions in New quantities/unitsN/A — no new quantities/units added. Generated codeN/A — no changes to Code quality / correctnessVerified the change is consistent with how Potential bugsNone found. The PerformanceNegligible — same single equality check as before. Test coverageGood coverage of the new boundary:
One optional suggestion: it might be worth a test (here or in a follow-up) exercising SecurityNo concerns. Overall: a clean, minimal, low-risk change that does what the description says. Good call extracting this out of #1544 for focused review. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1690 +/- ##
=======================================
Coverage 96% 96%
=======================================
Files 456 454 -2
Lines 30107 29497 -610
=======================================
- Hits 29043 28459 -584
+ Misses 1064 1038 -26
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Extracted from #1544 because representing a unit system for only the dimensions an application uses is useful independently of QuantityValue. Moving it out gives the API change focused review and reduces the original PR's scope.
Changes:
Tests: