NREL / openstudio-common-measures-gem

Other
8 stars 15 forks source link

Adding EV charging related measures #30

Closed amyeallen1 closed 4 years ago

amyeallen1 commented 4 years ago

Hi Nick and Katherine, David suggested that I submit these two EV charging related measures that I wrote to this repo. Shanti would like to make this work publicly available. @nllong @kflemin Do one of you two review measures for this repo or should I ask someone else? Thanks!

kflemin commented 4 years ago

@amyeallen1, some of your files are missing a blank line (you can see them if you click on your commit). Otherwise as long as you have tests and they pass you should be good to go. I'd have @nllong review this too.

amyeallen1 commented 4 years ago

Thanks, Katherine! @kflemin I think the flies that are tagged as missing the blank line were automatically generated from running the test on the example model file, such as the results.json. Should I include the results from the tests in the commit?

kflemin commented 4 years ago

@amyeallen1 no, I would not include test results in the commits. They should be ignored automatically via the .gitignore file. I'm not sure why they should up here.

nllong commented 4 years ago

I second @kflemin in not including the test results. You will have to do a weird work around to remove them from git (for real) since you have included them already. The approach would be to do the following:

amyeallen1 commented 4 years ago

Thanks Nick and Katherine! I just followed those instructions, Nick. @nllong @kflemin

amyeallen1 commented 4 years ago

Thanks, Nick and Katherine! @nllong @kflemin

amyeallen1 commented 4 years ago

Thanks, Nick, for reviewing and merging the PR! @nllong