-
Notifications
You must be signed in to change notification settings - Fork 390
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
Add units to RotationalStiffness, VolumePerLength, ForcePerLength #776
Add units to RotationalStiffness, VolumePerLength, ForcePerLength #776
Conversation
…itional prefixes to ForcePerLength.
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.
Verified all the conversions.
A couple suggestions to rename some units. Make sure to run generate-code.bat once more and you probably have to manually rename the properties in the test code.
Codecov Report
@@ Coverage Diff @@
## master #776 +/- ##
==========================================
+ Coverage 63.47% 63.82% +0.34%
==========================================
Files 278 278
Lines 41087 41477 +390
==========================================
+ Hits 26081 26471 +390
Misses 15006 15006
Continue to review full report at Codecov.
|
Thanks for tidying up the tests..looks much better! I updated the unit naming in the Rotational Stiffness as suggested. I also updated the names of some of the other units in the same file to keep everything following a consistent convention. |
@@ -22,8 +22,8 @@ | |||
] | |||
}, | |||
{ | |||
"SingularName": "PoundForceFootPerDegrees", | |||
"PluralName": "PoundForceFeetPerDegrees", | |||
"SingularName": "PoundForceFootPerDegree", |
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.
Great initiative, however this would be a breaking change and not something we can do.
I propose reverting these and adding a comment in #563 to address it there.
@@ -3,7 +3,7 @@ | |||
"BaseUnit": "CubicMeterPerMeter", | |||
"XmlDoc": "Volume, typically of fluid, that a container can hold within a unit of length.", | |||
"BaseDimensions": { | |||
"L": 3 | |||
"L": 2 |
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.
Good catch
Thanks a lot, a new nuget is on the way. |
Great, thanks a lot. I will make a note in the linked issue as requested. |
In addition to adding the additional units I also made a change in the VolumePerLength BaseDimensions to change it from L=3 to L=2. I'm not sure if I misunderstood something, but if its m^3 / m then I'm not sure how it can be L=3. Maybe you did this to avoid ambiguity with area? I checked in the original PR (#604) and cannot find any explanation why its like this.