Skip to content

Conversation

vgreen-BH
Copy link
Contributor

NOTE: Depends on

Issues addressed by this PR

Closes #89

Adds two new mullion systems (Kawneer 2500, Kawneer 1600) to the facade mullion database.

@vgreen-BH vgreen-BH requested a review from enarhi May 28, 2021 00:27
Copy link
Member

@enarhi enarhi left a comment

Choose a reason for hiding this comment

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

@vgreen-BH looks like the last entry cam through without a name:
image
Could you also please rename the dataset to Mullions(Kawneer), and rename the JSON to match? Aside from that it looks good, if you can just update to fix those bits then I'll go ahead and rereview.

Fixed missing name on last mullion item. Renamed dataset and JSON to "Mullions(Kawneer)"
@enarhi
Copy link
Member

enarhi commented May 28, 2021

@vgreen-BH looks the 1600's have a units issues as they are substantially larger than the others (about 9' deep!). Can you update those with the right scaling please?

Fixed size units for Kawneer 1600 Mullions
@enarhi enarhi self-requested a review June 7, 2021 22:10
Copy link
Member

@enarhi enarhi left a comment

Choose a reason for hiding this comment

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

LGTM!

@enarhi enarhi added the type:feature New capability or enhancement label Jun 8, 2021
@al-fisher al-fisher merged commit 4d02eb2 into master Jun 9, 2021
@al-fisher al-fisher deleted the BHoM_Datasets-#89-UpdateMullionDataset branch June 9, 2021 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Mullion Dataset with New Systems
3 participants