Skip to content

Conversation

@markborkum
Copy link
Contributor

Any background context you want to provide?

See proposal.

What does this PR do?

See proposal.

How should this be manually tested?

See proposal.

What are the relevant tickets?

N/A

Screenshots (if appropriate)

N/A

BuildingSync.xsd Outdated
</xs:element>
<xs:element ref="auc:LinkedPremises" minOccurs="0"/>
<xs:element ref="auc:UserDefinedFields" minOccurs="0"/>
<xs:element name="AirInfiltrationCondition" type="auc:EquipmentCondition" minOccurs="0"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Mark, is there a justification of choosing to add specific XXXCondition for every system over using the generic EquipmentCondition in your previous commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To pass the tests, I followed the naming convention that was established for other "XXXCondition" elements.

Copy link
Contributor

@JieXiong9119 JieXiong9119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend more consistent naming for EquipmentCondition elements, but let me know if there is a specific naming you would prefer unchanged.
PS: The HVACControlSystemCondition is a typo I believe.

Copy link
Contributor

@JieXiong9119 JieXiong9119 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a slight change. All seem good to me!

<xs:element ref="auc:LinkedPremises" minOccurs="0"/>
<xs:element ref="auc:UserDefinedFields" minOccurs="0"/>
<xs:element ref="auc:Quantity" minOccurs="0"/>
<xs:element name="HVACSystemCondition" type="auc:EquipmentCondition" minOccurs="0"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be HVACSystemCondition instead of HVACControlCondition

@JieXiong9119
Copy link
Contributor

@markborkum Do you want to chat on more details on this PR or can we merge it directly?

@markborkum
Copy link
Contributor Author

@markborkum Do you want to chat on more details on this PR or can we merge it directly?

We can merge it. Thanks, @JieXiong9119.

@JieXiong9119
Copy link
Contributor

merging this

@JieXiong9119 JieXiong9119 merged commit 2d1e04c into develop-v2 Apr 18, 2025
3 checks passed
@JieXiong9119 JieXiong9119 deleted the proposal/2025-add-core-elements-to-all-assets branch April 18, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Non-breaking Change Schema: General General update to BuildingSync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants