Repository navigation
Follow cross-references in device type tables - #90
Conversation
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
AryaHassanli
left a comment
There was a problem hiding this comment.
Thanks for this — unifying the ID/Name pairs into EntityReference is a nice simplification, and having anchors inside table rows resolve to the row's entity is a welcome fix.
I built base (c2fc3ce) and head (c39f994), ran go test ./... (all pass) and staticcheck, then ran alchemy dm --force -a in-progress from both builds on current spec master and diffed the output and logs. This turned up a few regressions the tests don't catch:
Blocking (seen in the A/B run)
- Tag requirements: 16 new fatal errors, e.g.
BatteryStorage.adoc:248 - mismatched namespace name in tag requirement: 0x0F vs. Power Source(also HeatPump, ElectricalEnergyTariff, ElectricalDistributionEnclosure).TableColumnNamespaceIDis passed as a name column, and each error is reported twice. - Dropped header variants:
- Cluster requirements no longer read
Cluster Name, so the templated RM device types (HepaFilter, ActivatedCarbonFilter, WaterTankLevel) now generate<cluster id="{RM_CL_ID}" name="" ...>. - Element requirements now read only
Cluster Name, but the spec header isCluster, so element-requirement cluster names and xrefs are lost completely.
- Cluster requirements no longer read
Likely bugs
3. ElementRequirement.Clone() no longer copies Element (used for composed device types in composition.go).
4. ElementRequirement.Equals no longer compares Element.
5. Possible nil-pointer panic in EntityReference.Equals / ElementReference.Equals when only one side has an XRef (typed-nil passed into CrossReference.Equals).
6. Three swallowed errors in requirements.go (staticcheck SA4006 on lines 38, 255, 279).
7. dm composed path now passes deviceType instead of dt to renderClusterRequirement.
8. DeviceTypeReferenceTypeMismatch is built with a nil DeviceType and with the target entity as Source, because entity is shadowed.
Smaller points are in the inline comments. Once fixed, a dm A/B on the current spec should show zero diff apart from any intended changes. It would also be great to add tests for xref resolution, the new mismatch errors, and anchor-in-table-row lookup.
|
Thank you @hasty for addressing the comments. |
Yeah, auto-complete changed it on me without me noticing. Fixed. |
This unifies the general parsing of entity ID & Name pairs in device types to allow for references instead of names. It also fixes the association of anchors within table rows to the entities defined in those rows (e.g. Conditions).
It adds several new error types to deal with mismatches between reference targets and the IDs and names in those tables.