fix edge case duration roundings #2208
Conversation
|
/cc @myronmarston |
| minutes = (duration.to_i / 60).to_i | ||
| seconds = duration - minutes * 60 | ||
| minutes = (duration.round / 60).to_i | ||
| seconds = (duration - minutes * 60).abs |
There was a problem hiding this comment.
Why is the abs needed? I'm trying to imagine a case where the expression in parens returns a negative value.
There was a problem hiding this comment.
359.999999999999 - 6*60 is negative
There was a problem hiding this comment.
But in that case isn't it just rounded off to 0? When I see abs it makes me think that we're able to get values like -7 and then the abs flips it to 7...which doesn't make much sense.
Can we just use .round instead? That would be less confusing if it works.
|
@myronmarston I removed the |
|
LGTM apart from the Travis failure. |
|
So it turns out |
|
Green, final review @myronmarston ? |
|
LGTM |
fix edge case duration roundings
fix edge case duration roundings
fix edge case duration roundings
fix edge case duration roundings --- This commit was imported from rspec/rspec-core@896c8da.
Based on the work of @Fabersky but with @myronmarston's spec. Supersedes and fixes #2187.