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

Don't cast nanoseconds to integers when writing output #1876

Merged
merged 1 commit into from
Mar 8, 2020
Merged

Don't cast nanoseconds to integers when writing output #1876

merged 1 commit into from
Mar 8, 2020

Conversation

khyperia
Copy link
Contributor

@khyperia khyperia commented Feb 26, 2020

I mostly care about catch_reporter_xml.cpp, but, figured I'd also do catch_reporter_console.cpp while I'm in the area.

Also, some context, since it's not plainly visible in the diff - stdev (just below) is already being written as doubles, without casting to int, so I don't think there's an issue with mean being written as a double as well

@codecov
Copy link

codecov bot commented Feb 26, 2020

Codecov Report

Merging #1876 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1876   +/-   ##
=======================================
  Coverage   86.48%   86.48%           
=======================================
  Files         131      131           
  Lines        3899     3899           
=======================================
  Hits         3372     3372           
  Misses        527      527           

@horenmar
Copy link
Member

horenmar commented Mar 8, 2020

I had some misgivings about the resulting formatting for ConsoleReporter (I don't want to tell users that something took 1.0000002213 ns), but it looks fine and there probably should be a more significant cleanup of how the times are written if I want it to be human readable.

@horenmar horenmar merged commit 022b61f into catchorg:master Mar 8, 2020
@khyperia khyperia deleted the dont-cast-output branch March 9, 2020 08:42
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