-
Notifications
You must be signed in to change notification settings - Fork 387
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
Rename SpecificFuelConsumption.GramPerKiloNewtonSecond to GramPerKilonewtonSecond #1491
Rename SpecificFuelConsumption.GramPerKiloNewtonSecond to GramPerKilonewtonSecond #1491
Conversation
Common/UnitEnumValues.g.json
Outdated
"PoundMassPerPoundForceHour": 4 | ||
"PoundMassPerPoundForceHour": 4, | ||
"GramPerKilonewtonSecond": 11, | ||
"KilogramPerKilonewtonSecond": 6 |
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.
Same here:
This maybe a little hack-y, but I think that if we changed these by hand (to the old values) - I think they would stick.
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.
Yup, I suggest we do the same here, match the old values.
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.
fixed in 730eab1
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.
One suggestion, other than that ready to merge
Common/UnitEnumValues.g.json
Outdated
"PoundMassPerPoundForceHour": 4 | ||
"PoundMassPerPoundForceHour": 4, | ||
"GramPerKilonewtonSecond": 11, | ||
"KilogramPerKilonewtonSecond": 6 |
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.
Yup, I suggest we do the same here, match the old values.
Hmm, I just realized that in my other project I actually have the SI coverage tests [Skipped] for this one.. As you've correctly observed in #1473 the {
"SingularName": "GramPerKilonewtonSecond",
"PluralName": "GramsPerKilonewtonSecond",
"BaseUnits": {
"L": "Meter",
"T": "Second"
},
"FromUnitToBaseFunc": "{x}",
"FromBaseToUnitFunc": "{x}",
"Prefixes": [ "Kilo" ],
"Localization": [
{
"Culture": "en-US",
"Abbreviations": [ "g/(kN�s)" ],
"AbbreviationsForPrefixes": { "Kilo": [ "kg/(kN�s)" ] }
}
]
} However this showcases another limitation of using the The problem is even worse, when this is the {
"SingularName": "GramPerNewtonSecond",
"PluralName": "GramsPerNewtonSecond",
"BaseUnits": {
"L": "Meter",
"T": "Second"
},
"FromUnitToBaseFunc": "{x} * 1000",
"FromBaseToUnitFunc": "{x} / 1000",
"Prefixes": [ "Kilo" ],
"Localization": [
{
"Culture": "en-US",
"Abbreviations": [ "g/(N�s)" ],
"AbbreviationsForPrefixes": { "Kilo": [ "kg/(N�s)" ] }
}
]
} This still wouldn't give us the proper SI unit, as the The only way this can be made right is to have the PS A little more of this and the doctor's diagnosis is going to be |
Oh I think I made a mistake here- we won't actually need |
Ok, I think I almost got it figured out- for the This is because the |
Good, it seems this wasn't all in vain - #1492 turned up with quite a few new options for the Anyway, this PR was only about renaming the unit, so it should be good to go. For the rest of it, I'll open another one later.. |
Rename
SpecificFuelConsumption.GramPerKiloNewtonSecond
toGramPerKilonewtonSecond
(part of #1200)