NREL / openstudio-common-measures-gem

Other
7 stars 15 forks source link

envelope_and_internal_load_breakdown syntax error #68

Closed shorowit closed 3 years ago

shorowit commented 3 years ago

Multiple users hitting this:

The measure is definitely broken, there is a rescue (here) without an accompanying begin.

shorowit commented 3 years ago

Also, how could the tests for this measure possibly work?

DavidGoldwasser commented 3 years ago

@shorowit I'll look into those posts more, but that change in how rescue was added by code cleanup I think when we went to Ruby 2.5, here is a link to old code. The rescue code does work this way. I will try to look at what is happening with those posts and add new tests for them. https://github.com/NREL/openstudio-common-measures-gem/blame/ed1ec3406dffbb2fd2460264bc737c6ca0f59201/lib/measures/envelope_and_internal_load_breakdown/measure.rb#L183-L220

DavidGoldwasser commented 3 years ago

@shorowit one of them was messed up download from BCL, which was fixed by throwing away and re-downloading, the other is trying to run the measure in 2.x, which uses older Ruby. I probably just need to upgrade this to 3.0 or even 3.1 or higher (whenever the last time sql changes were made in E+ that this uses). People using older OS will have to get the measures from older measure gem releases.

shorowit commented 3 years ago

Thanks @DavidGoldwasser, I stand corrected. Apparently the rescue statement in ruby doesn't always require a begin. So I guess the code is fine and, as you said, the measures just need to be updated to have a minimum version so they aren't used with an incompatible ruby/OS.

DavidGoldwasser commented 3 years ago

Related to issue #52