search

serokell / o-clock

:hourglass: Type-safe time units in Haskell
Mozilla Public License 2.0
49 stars 6 forks source link

Show/Read instances for Time now use mixed fractions #75

Closed adituv closed 6 years ago

adituv commented 6 years ago

The Show and Read instances for Time now support mixed fractions: show always outputs a mixed fraction, and read can read either an improper fraction (the previous functionality) or a mixed fraction.

The Show instance now has showsPrec explicitly implemented instead of show as precedence could potentially matter with the '+' in the middle. Parentheses are added when the surrounding precedence is greater than 6, the precedence of '+'. For example, 3 * (7+2/5s). This should not affect seriesF as that uses show, and so a precdence of 0.

Resolves #61

adituv commented 6 years ago

Due to #60 (and because all the Show tests are implicit in seriesF tests) I haven't implemented tests for the Show instance explicitly. Let me know if you would like me to do that too, or whether you would prefer to wait on #60.

I tried to look at #60 but I don't understand the 8.0.4+ type logic enough to handle it right now

chshersh commented 6 years ago

Oh, one more thing: could you please update changelog as well?

adituv commented 6 years ago

Sorry, I seem to have messed up something on git! I'm fixing it now.

adituv commented 6 years ago

Git problems resolved. I am not sure on the protocol for when responding to requested changes though: should I squash the commits? For now I've left them separated.

chshersh commented 6 years ago

@adituv Sorry, we don't have contributing guide at this project at the moment 😞 So it's up to you. It makes sense to me to squash commits after code review (to make code review easier and resulting history cleaner). But I don't insist on this for volunteer contributors 🙂

chshersh commented 6 years ago

@adituv Thanks for your amazing contribution to our project! 🎉