Skip to content

Conversation

tongbong
Copy link
Contributor

No description provided.

tongbong added 4 commits January 21, 2016 12:31
Tests do not pass
Change base unit to radian per second because of loss of precision
(tests were red).
Add prefixes for radian per second.
@angularsen
Copy link
Owner

This looks good to me, the only concern is changing the base unit. This has not been done before and it could potentially break backwards compatibility with serialization, but I don't believe it should in practice at least not for the Json serialization in this repo, since it serializes both value and unit. Can you think of any other scenario? If not I'll merge this in.

@angularsen
Copy link
Owner

After some thought I am fine with changing the base unit, and realized we have in fact done similar before. Units.NET does not guarantee this will not change and this is mentioned in the README section on serialization. I'll add a text to the README explicitly stating that the base unit may change and should not be used for serialization.

angularsen added a commit that referenced this pull request Jan 23, 2016
…peed_WithCorrection

Add RotationalSpeed units
@angularsen angularsen merged commit 5993e26 into angularsen:master Jan 23, 2016
@tongbong
Copy link
Contributor Author

Yes, at the time I wasn't too happy either to change the base unit, but thought that precision is important and tests were green. I agree that serializing both value and unit will avoid any trouble with change base units.

@tongbong tongbong deleted the feature/add-RotationalSpeed_WithCorrection branch January 25, 2016 13:17
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