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.

Sign up for GitHub

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

Jump to bottom

Delete unused protected parameters ndim, ndim2 and ndim_pointGravity #4266

Merged
merged 3 commits into from
Jan 18, 2024
Merged

Delete unused protected parameters ndim, ndim2 and ndim_pointGravity #4266

merged 3 commits into from
Jan 18, 2024

Conversation

tobolar
Copy link
Contributor

@tobolar tobolar commented Jan 16, 2024

Close #4246

IMO no conversion script is needed for protected parameters.

@tobolar tobolar added the L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody label Jan 16, 2024
beutlich
beutlich reviewed Jan 16, 2024
View reviewed changes
Copy link
Member

@beutlich beutlich left a 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.

@tobolar
Copy link
Contributor Author

tobolar commented Jan 17, 2024 •
edited

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 package.mo later on.

HansOlsson
HansOlsson approved these changes Jan 17, 2024
View reviewed changes
Copy link
Contributor

@HansOlsson HansOlsson left a 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.

@beutlich
Copy link
Member

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 package.mo later on.

No, just do it here.

@Harisankar-Allimangalath
Copy link
Contributor

@beutlich can you please review the changes done by @tobolar , the milestone part is missing here and I hope its ok to go with MSL4.1.0 . Thankyou

beutlich
beutlich reviewed Jan 18, 2024
View reviewed changes
Modelica/package.mo Outdated Show resolved Hide resolved
beutlich
beutlich approved these changes Jan 18, 2024
View reviewed changes
Copy link
Member

@beutlich beutlich left a 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).

@Harisankar-Allimangalath @beutlich
Update Modelica/package.mo
0368d77 
Co-authored-by: Thomas Beutlich <modelica@tbeu.de>
@Harisankar-Allimangalath
Copy link
Contributor

@MartinOtter and @tobolar can this PR have the milestone addressing to MSl4.1.0.

@Harisankar-Allimangalath
Copy link
Contributor

Fine for me (if release notes are slighty reformulated).

@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?

@MartinOtter MartinOtter added this to the MSL4.1.0 milestone Jan 18, 2024
@tobolar
Copy link
Contributor Author

tobolar commented Jan 18, 2024 •
edited

Fine for me (if release notes are slighty reformulated).

@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.

@MartinOtter and @tobolar can this PR have the milestone addressing to MSl4.1.0.

I'm in favor.

@beutlich
Copy link
Member

Fine for me (if release notes are slighty reformulated).

@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?

No, I only meant 0368d77

@Harisankar-Allimangalath Harisankar-Allimangalath merged commit e041c72 into modelica:master Jan 18, 2024
2 checks passed
@tobolar tobolar deleted the fix4246_unused_ndim_world branch January 18, 2024 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MartinOtter MartinOtter MartinOtter approved these changes

@HansOlsson HansOlsson HansOlsson approved these changes

@beutlich beutlich beutlich approved these changes

Assignees
No one assigned
Labels
L: Mechanics.MultiBody Issue addresses Modelica.Mechanics.MultiBody
Projects
None yet
Milestone
MSL4.1.0
Development

Successfully merging this pull request may close these issues.

Unused ndim in MBS World
5 participants
@tobolar @beutlich @Harisankar-Allimangalath @MartinOtter @HansOlsson