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

fix Num multiplication, add Real, Enum, Integral instances #67

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

Mathnerd314
Copy link
Contributor

These instances are based on using the nanosecond as the underlying unit, i.e. the value of a TimeSpec is the integer s * 1e9 + ns.
Before it was based on representing the rational s + (ns / 1e9). But the integer valuation satisfies the ring laws and provides much more intuitive behavior, e.g.

(2 :: TimeSpec)*(2 :: TimeSpec) = (4 :: TimeSpec)

Before, this gave 0.

This should properly fix #32, #33. Of course it breaks anyone who was using the old version of (*). The old version of (*) is equivalent to a * b `div` 1000000000 in the new version.

These instances are based on using the nanosecond as the underlying unit,
i.e. the value of a TimeSpec is the integer s * 1e9 + ns.
Before it was based on representing the rational s + (ns / 1e9).
But the integer valuation satisfies the ring
laws and provides much more intuitive behavior, e.g.
(2 :: TimeSpec)*(2 :: TimeSpec) = (4 :: TimeSpec)
Before, this gave 0.
@CetinSert CetinSert merged commit bc5a1b6 into corsis:master Jul 13, 2021
@dten
Copy link

dten commented Jul 2, 2022

This change broke our program. I blame ourselves for multiplying two TimeSpecs together in the first place (no one was after seconds-squared). But this is a breaking change and probably should have at least made it to the changelog

@Mathnerd314
Copy link
Contributor Author

Well, it is in the changelog, somewhat vaguely, as "TODO: new module System.Clock.Seconds" and following. I think the more pressing issue is that per https://pvp.haskell.org/ the changes should have caused a major version bump, so the release should be 0.9.0.

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.

(*) for TimeSpec doesn't follow the ring laws
3 participants