-
Notifications
You must be signed in to change notification settings - Fork 169
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
Delete unused protected parameters ndim, ndim2 and ndim_pointGravity #4266
Delete unused protected parameters ndim, ndim2 and ndim_pointGravity #4266
Conversation
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.
If you like you can add it to the list of changed components (breakng compatibilty) already now, see #4260.
@beutlich Shall I better rebase this PR to beutlich:create_Version_4_1_0? It could make it easier to merge |
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.
I agree that an unused protected parameter can be removed, even if someone in theory could have used it.
No, just do it here. |
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.
Fine for me (if release notes are slighty reformulated).
Co-authored-by: Thomas Beutlich <[email protected]>
@MartinOtter and @tobolar can this PR have the milestone addressing to MSl4.1.0. |
@beutlich Are you referring to the auto generation of release notes? The implication you refer is not clear to us, so could you please clarify? |
I think @beutlich means his suggestions #4266 (comment). You have already merged this.
I'm in favor. |
Close #4246
IMO no conversion script is needed for protected parameters.