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

Consider using default value instead of hardcoded '-' in MetricColumn.GetValue() #1168

Closed
sleemer opened this issue May 25, 2019 · 4 comments
Milestone

Comments

@sleemer
Copy link
Contributor

sleemer commented May 25, 2019

What we have currently:

 Method |       Mean |    StdDev |  Gen 0 | Allocated |
---------- |----------- |---------- |------- |---------- |
 Iterative | 31.0739 ns | 0.1091 ns |      - |       0 B |
      LINQ | 83.0435 ns | 1.0103 ns | 0.0069 |      32 B | 

What we need:

 Method |       Mean |    StdDev |  Gen 0 | Allocated |
---------- |----------- |---------- |------- |---------- |
 Iterative | 31.0739 ns | 0.1091 ns | 0.0000 |       0 B |
      LINQ | 83.0435 ns | 1.0103 ns | 0.0069 |      32 B | 

Why we need it:
We run benchmarks as a step of our TeamCity build. And having consistent csv report file will simplify exporting results into influxdb using csv plugin for telegraf.

@AndreyAkinshin
Copy link
Member

@sleemer thanks for the request!
@adamsitnik what do you think about it?

@adamsitnik
Copy link
Member

I am ok with changing the CSV file output to be more consistent, but I would prefer not to change the way we display the results in the console/GH/HTML. I like the fact that we display - - it's more obvious and easier to read (I don't need to look at the digits)

@AndreyAkinshin
Copy link
Member

@sleemer is it OK if we change the default value only in the CSV reports?

@sleemer
Copy link
Contributor Author

sleemer commented May 30, 2019

@AndreyAkinshin, absolutely! That’s exactly what I meant. Sorry, if it wasn’t clear from my original proposal.

sleemer added a commit to sleemer/BenchmarkDotNet that referenced this issue Sep 3, 2019
- CsvExporter now prints '0' instead of '-' in the report
- Add PrintZeroValuesInContent option in SummaryStyle to control it (by default it is false)
- Add tests to cover changed behavior

Closes dotnet#1168
@AndreyAkinshin AndreyAkinshin added this to the v0.11.6 milestone Sep 6, 2019
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

No branches or pull requests

3 participants