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

Global state refactor continued #8414

Merged
merged 91 commits into from
Dec 15, 2020
Merged

Conversation

Myoldmopar
Copy link
Member

Pull request overview

  • This gets the number of global variables hanging out there down to around 5200, down from like 7000 before this branch!
  • A vast majority of the global variables were actually integers and characters hanging around that were converted into constexpr and enums wherever possible.
  • There will be an upcoming branch where the state argument is reworked, this branch does not address that, just stay tuned.

This is a big set of changes, but I think it will be totally clean on CI.

mitchute and others added 30 commits December 1, 2020 14:08
@Myoldmopar Myoldmopar added the Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring label Dec 9, 2020
@mitchute mitchute added this to the EnergyPlus Future milestone Dec 10, 2020
@Myoldmopar
Copy link
Member Author

Only CI issues are now the spurious diffs in the shading file and the integration coverage dipping below the limit. This looks to be ready to go. I will hold on merging momentarily while I do final review on the air wall PR and possibly drop that in first. But as long as that one is good to go then these will both drop in sometime today.

@Myoldmopar
Copy link
Member Author

Alright got develop pulled in, everything is looking great here locally. Enabled the unit test that was failing as well. If CI is clean, this beast can drop in.

@Myoldmopar
Copy link
Member Author

OK, this looks good to go in. This is a big set of changes, and there will be a follow-on branch that addresses the state-as-an-argument issue. If you are having lots of conflicts, just let me know and I'll pull your branch and help get them addressed. I'll be dropping this in now... Thanks @mitchute and @jmythms for the big effort here. Among all the actual global "variables" that were addressed, another 400 or so int const variables were converted to enums. 💯

@Myoldmopar Myoldmopar merged commit f04ed6d into NREL:develop Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactoring Includes code changes that don't change the functionality of the program, just perform refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants