added speed units #346
added speed units #346
Conversation
FootPerMinute FootPerHour UsSurveyFootPerSecond UsSurveyFootPerMinute UsSurveyFootPerHour InchPerSecond InchPerMinute InchPerHour YardPerSecond YardPerMinute YardPerHour
|
@ferittuncer Want to review this one? |
|
I started to review @angularsen @gojanpaolo |
| protected override double UsSurveyFeetPerMinuteInOneMeterPerSecond => 196.85; | ||
|
|
||
| protected override double UsSurveyFeetPerHourInOneMeterPerSecond => 11811; | ||
|
|
There was a problem hiding this comment.
We prefer using scientific notation in tests. For example, 1.1811E4 instead of 11811.
Rest is fine, I found no errors in quantity definitions and test values. Good job 🥇
There was a problem hiding this comment.
Thanks, but I guess you forgot 10 other tests?
There was a problem hiding this comment.
Oh sorry about that. I'm looking at the other tests and can't seem to figure out the convention. (e.g. RotationalSpeedTests, PressureTests)
There was a problem hiding this comment.
No you misunderstood. You have added 11 new units for Speed quantity, so you wrote 11 tests, one for each.
In your last commit you changed the test of UsSurveyFeetPerHourInOneMeterPerSecond into scientific notation.
I meant that you forgot to change the other tests for the units which you added, which includes Foot, UsSurveyFoot, Yard and Inch units.
Did I express myself clearly?
There was a problem hiding this comment.
Well it's not critical, let's not wait for this, we can return and change them into scientific notation later on. I'm merging this pull request. @angularsen
| protected override double UsSurveyFeetPerMinuteInOneMeterPerSecond => 196.85; | ||
|
|
||
| protected override double UsSurveyFeetPerHourInOneMeterPerSecond => 11811; | ||
|
|
There was a problem hiding this comment.
Thanks, but I guess you forgot 10 other tests?
|
Good job guys, looks good to me! Happy holidays! |
|
@ferittuncer You can go ahead and do the release whenever is convenient for you. |
|
@angularsen @gojanpaolo Happy holidays! |
3 participants
Fixes #345
FootPerMinute
FootPerHour
UsSurveyFootPerSecond
UsSurveyFootPerMinute
UsSurveyFootPerHour
InchPerSecond
InchPerMinute
InchPerHour
YardPerSecond
YardPerMinute
YardPerHour