Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow additional floor types #332

Merged
merged 3 commits into from
Nov 7, 2022
Merged

Allow additional floor types #332

merged 3 commits into from
Nov 7, 2022

Conversation

bpark1327
Copy link
Contributor

@bpark1327 bpark1327 commented Jul 14, 2022

Fixes #321

Renames FrameFloors/FrameFloor to Floors/Floor.
Renames WallType/StructurallyInsulatedPanel to WallType/StructuralInsulatedPanel
Adds FloorType with choices of:

  • WoodFrame
  • StructuralInsulatedPanel
  • SteelFrame
  • SolidConcrete
  • Other

@bpark1327
Copy link
Contributor Author

bpark1327 commented Jul 14, 2022

Some additional thoughts (#327 (review)):

@shorowit shorowit added the merge label Oct 11, 2022
@shorowit shorowit marked this pull request as ready for review October 11, 2022 20:35
@RockyMtnBrian
Copy link

Note that "SIP" stands for Structural Insulated Panel, as opposed to "structurally". If feasible, I suggest eliminating the "ly" part of the FloorType label.

…FloorType element that references the new FloorType data type. Remove dumb annotations.
@shorowit
Copy link
Contributor

Thanks @RockyMtnBrian, I have fixed this. Because that name was already used for WallType, it's a breaking change.

I also discovered that @bpark1327 had added a new data type for floor type, but didn't actually add a FloorType element under Floor that uses it. That's now fixed too.

@RockyMtnBrian
Copy link

Just a thought, @shorowit: If there's a need to separately label the SIP wall vs SIP floor, then SIPWall and SIPFloor might do the job.

@RockyMtnBrian
Copy link

Note that SIPs get used for roofs, too. SIPRoof?

@shorowit
Copy link
Contributor

@RockyMtnBrian There is not a need to distinguish SIP wall from SIP floor -- one will show up as Wall[WallType/StructuralInsulatedPanel] and the other will show up as Floor[FloorType/StructuralInsulatedPanel].

For roofs, it looks like the best place to add a SIP option under Roof would be in the DeckType element. (RoofType would be most consistent with WallType and FloorType, but unfortunately it's already used to describe the roofing material.)

@nmerket nmerket merged commit 865f5f8 into master Nov 7, 2022
@nmerket nmerket deleted the additional_floor_types branch November 7, 2022 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow describing mass floors
4 participants