search

NREL / openstudio-model-articulation-gem

Other
7 stars 9 forks source link

add empd material properties measure #73

Closed GFlechas closed 3 years ago

GFlechas commented 3 years ago

addresses issue #72. New measure to add empd material properties to an OpenStudio model.

DavidGoldwasser commented 3 years ago

@GFlechas, Just wanted to check if you have sent in a code contribution email yet for this repository. If not this week we can try to review and merge this in next week.

GFlechas commented 3 years ago

I think everything is good. I have run code cleanup on my PR. I'm a git novice so if I've made a mistake please let me know and I'll get it fixed!

I will get the contribution agreement submitted shortly.

Thank you!

DavidGoldwasser commented 3 years ago

@GFlechas the variable names were changed in the measure.rb but not in the test. You can change these here, then update the measure.xml again and should be ready. https://github.com/GFlechas/openstudio-model-articulation-gem/blob/develop/lib/measures/add_empd_material_properties/tests/add_empd_material_properties_test.rb#L83-L92

I tried to do it myself, but my attempt broke something in our CI, maybe we will have better luck if it is fixed in your fork and on PR #73. https://github.com/NREL/openstudio-model-articulation-gem/pull/81

David

DavidGoldwasser commented 3 years ago

The CI on other branch worked now which closed this PR.

GFlechas commented 3 years ago

Hey David, it's been a hectic past week. Wanted to check in and make sure things are squared away. It looks like the issue resolved, but I want to verify you don't still need something from my end. Thanks for all your help!