-
Notifications
You must be signed in to change notification settings - Fork 84
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
Issue1781 port placement #1782
Issue1781 port placement #1782
Conversation
@FWuellhorst : Can you please do a review of this feature so we have a non-LBNL person reviewing it. Thanks. |
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.
Looks good, nice feature.
I have some comments / questions. Moving to protected would be a viable option, I think.
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.
Thanks for the changes, looks really good!
@mwetter This is ready to merge. |
This is done manually as git apply with diff from d2edb0c fails. error: IBPSA/Fluid/Interfaces/PartialTwoPort.mo: patch does not apply
Revert most of the changes from #1782
This closes #1781.
I also proposed in commit 60c19a7 to modify port placement in
Fluid/Storage/BaseClasses/PartialStratified.mo
. Although it may annoy a few users (whose model diagrams will be modified) it will benefit many future users (with a more intuitive icon layout). But I can also revert that change if the disturbance is too big.