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

Fix illegal ModelicaTest.Media.TestOnly.WaterIF97_dewEnthalpy #4429

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

casella
Copy link
Contributor

@casella casella commented Jun 20, 2024

ModelicaTest.Media.TestOnly.WaterIF97_dewEnthalpy used the partial medium Modelica.Media.Water.WaterIF97_fixedregion, which is not allowed by the MLS, so it is not valid Modelica code.

This PR replaces the medium model with the non-partial package Modelica.Media.Water.StandardWater

ModelicaTest.Media.TestOnly.WaterIF97_dewEnthalpy used the partial medium Modelica.Media.Water.WaterIF97_fixedregion, which is not allowed by the MLS, so it is not valid Modelica code.

This PR replaces the medium model with the non-partial package Modelica.Media.Water.StandardWater
@casella
Copy link
Contributor Author

casella commented Jun 20, 2024

After accepting it, this should be back-ported to maint/4.1.0

@beutlich beutlich added the L: ModelicaTest Issue addresses ModelicaTest, ModelicaTestConversion4 or ModelicaTestOverdetermined label Jun 21, 2024
Copy link
Contributor

@maltelenz maltelenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about Media to tell if this still tests the intended thing.

I do agree that it was illegal before and is now legal.

@casella
Copy link
Contributor Author

casella commented Jun 28, 2024

@maltelenz if you can approve this, we can merge it in. Thanks!

Copy link
Contributor

@maltelenz maltelenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@casella seems content with me approving even after my statement that I don't know a lot about Media, so approving.

@casella casella merged commit 97d66d3 into master Jul 1, 2024
2 checks passed
@casella casella deleted the casella-patch-1 branch July 1, 2024 06:35
@casella
Copy link
Contributor Author

casella commented Jul 1, 2024

@maltelenz the only knowledgeable people on Media that are available now are myself and @arunkumar-narasimhan. @rfranke resigned. @hubertus65 also is very competent, but he delegated to Arun because he doesn't have the time. So, I need someone else to rubber-stamp our PRs 😄

@beutlich beutlich changed the title Fixed illegal ModelicaTest.Media.TestOnly.WaterIF97_dewEnthalpy Fix illegal ModelicaTest.Media.TestOnly.WaterIF97_dewEnthalpy Jul 19, 2024
@beutlich beutlich added this to the maintenance milestone Jul 23, 2024
@Esther-Devakirubai
Copy link
Contributor

Backported to maintenance branch by #4437

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: ModelicaTest Issue addresses ModelicaTest, ModelicaTestConversion4 or ModelicaTestOverdetermined
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants