-
Notifications
You must be signed in to change notification settings - Fork 7
Fix include issue with Branch.hpp (#57) #58
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
Conversation
|
This should fix the mis-compilation being experienced in the CI of #32 |
pelesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Power flow branch header Branch.hpp is missing forward declaration to BranchData class. Adding the forward declaration should fix this issue.
The currently proposed solution introduces additional build dependencies that would be good to avoid.
@pelesh for future reference can you explain what you mean by this? |
In header files use forward declarations instead of including other headers whenever possible. That way, we have better control over what is included in each compilation unit. |
82ff81c to
e95dea6
Compare
|
Changed to adding the forward declaration instead of including the definition. In the process I realized the phasor dynamics implementation already had this forward declaration, so we should be all good there. @pelesh this forward declaration philosophy might be a good thing to put in the developer guidelines, since it seems like an easy thing to catch people unaware. |
pelesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to merge, thanks!
Fixes #57
Also brought some adjacent includes in line with style guidelines