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

Flow, revolution, Pressure, Localization, Unittests #7

Merged
merged 4 commits into from
Nov 12, 2013

Conversation

Tirael
Copy link
Contributor

@Tirael Tirael commented Nov 12, 2013

Hi!

Added Flow, Revolution, changed Pressure, Unit, UnitConverter, UnitSystem (add Russian and invariant localization).
Add unittests for Flow, Revolution, change unittests for Pressure, UnitConverterTests, UnitValueTests.
All tests passed.

@angularsen
Copy link
Owner

Thanks George, looks good. Merged.

A few minor things I fixed:

  • Missing KFSC case in UnitConverter.TryConvertPressure()
  • Variable names in FlowTests and RevolutionTests using "meter"
  • .ToString() has changed for all classes (but was not yet pushed so you could not know)

Things I did not fix:

  • Missing most imperial units in UnitSystem localization, such as Yard and Foot.

Also, may I propose a change in name for Revolution?

For example AngularSpeed, RotationalSpeed or CyclicFrequency appear as good choices in my opinion. What do you think?

Sources:
http://en.wikipedia.org/wiki/Revolutions_per_minute
http://en.wikipedia.org/wiki/Rotational_speed

@angularsen angularsen merged commit 2256d07 into angularsen:master Nov 12, 2013
@Tirael
Copy link
Contributor Author

Tirael commented Nov 13, 2013

Yes, I agree that the name "Revolution" is wrong. "Rotational Speed" will be more correct.

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