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

Rounding inconsistency #38

Closed
villebro opened this issue Sep 1, 2019 · 1 comment · Fixed by #41
Closed

Rounding inconsistency #38

villebro opened this issue Sep 1, 2019 · 1 comment · Fixed by #41

Comments

@villebro
Copy link
Contributor

villebro commented Sep 1, 2019

When formatting 11999 milliseconds using default options, the result is 1m 60s (should obviously be 2m). Looking at the unit tests I also didn't quite understand why seconds round down

	t.is(prettyMilliseconds(33333, {secondsDecimalDigits: 0}), '33s');

but milliseconds round up:

	t.is(prettyMilliseconds(33.333, {millisecondsDecimalDigits: 0}), '34ms');

Is there some reason why seconds round as we expect them to (mathematically), but milliseconds seem to round up?

@sindresorhus
Copy link
Owner

When formatting 11999 milliseconds using default options, the result is 1m 60s (should obviously be 2m)

Good catch.

Is there some reason why seconds round as we expect them to (mathematically), but milliseconds seem to round up?

Not that I can think of. It should really use Math.round() instead of Match.ceil().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants