Skip to content

Conversation

@qlambert-pro
Copy link
Contributor

@qlambert-pro qlambert-pro commented Mar 27, 2023

The language design group has been trying to standardize the handling of units, so that literal values do not contribute to the unit of an expression in a way that would prevent further unit checking. We expect fixes similar to the one present in this PR to be necessary in the future.

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

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

@qlambert-pro
Copy link
Contributor Author

@HansOlsson @casella Can one of you confirm that this is the direction we are going and approve the PR?

@tobolar
Copy link
Contributor

tobolar commented Mar 28, 2023

@qlambert-pro Simply add the suggested guys to reviewers. (They would remove themself when not relevant.)

@qlambert-pro
Copy link
Contributor Author

@tobolar I don't think I have the rights to do that, unfortunately.

@maltelenz maltelenz requested review from HansOlsson and casella March 29, 2023 07:43
@tobolar
Copy link
Contributor

tobolar commented Mar 29, 2023

In the original code, a kind of magic number was used! ;-)

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.

@HansOlsson
Copy link
Contributor

In the original code, a kind of magic number was used! ;-)

Well, we had the constant on the line above - so it wasn't that magic.

@hubertus65 hubertus65 merged commit bca45a4 into modelica:master Apr 6, 2023
@beutlich beutlich removed the request for review from casella April 11, 2023 18:51
@beutlich beutlich added the L: Media Issue addresses Modelica.Media label Apr 11, 2023
@beutlich beutlich added this to the MSL4.1.0 milestone Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: Media Issue addresses Modelica.Media

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants