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

Added support for molar flow units #1196

Merged
merged 9 commits into from
Feb 15, 2023

Conversation

dcasaseca
Copy link
Contributor

No description provided.

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Good work, tons of new stuff here.
Some comments on precision in conversions mostly, other than that this looks good.

Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Outdated Show resolved Hide resolved
Common/UnitDefinitions/MolarFlow.json Show resolved Hide resolved
UnitsNet.Tests/CustomCode/MolarFlowTests.cs Outdated Show resolved Hide resolved
UnitsNet.Tests/CustomCode/MolarFlowTests.cs Outdated Show resolved Hide resolved
UnitsNet.Tests/CustomCode/MolarFlowTests.cs Outdated Show resolved Hide resolved
dcasaseca and others added 2 commits February 11, 2023 01:21
Improve precision of test case
Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Great, just a few more minor things

@dcasaseca
Copy link
Contributor Author

dcasaseca commented Feb 14, 2023

This is most likely failing because of the property marked as obsolete. I had to make a manual change after running the automatic code generation tool. The manual change was on "NumberToHeatTransferCoefficientExtensionsTest.g.cs" for a missing namespace (System.Obsolete)

@dcasaseca
Copy link
Contributor Author

@angularsen please have a look, this builds perfectly fine for me

There is no reason to annotate generated test methods with obsolete attributes. Only the types and methods related to the implementation of the unit needs it, not tests.
@angularsen
Copy link
Owner

@dcasaseca I pushed a fix, reverting the manual changes to generated code and instead fixed the code generator to not emit obsolete attribute for test code. This seemed like a mistake in the existing codegen regarding obsolete units.

@angularsen angularsen merged commit 230a059 into angularsen:master Feb 15, 2023
@angularsen
Copy link
Owner

Thank you for the effort, a lot of new stuff here!
Nuget should be out shortly.

Release UnitsNet/5.2.0 · angularsen/UnitsNet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants