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.

Sign up for GitHub

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

Jump to bottom

OUTPUT_FORMAT=csv option is missing from benchmark/common.js #7890

Closed
adrian-nitu-92 opened this issue Jul 27, 2016 · 8 comments
Closed

OUTPUT_FORMAT=csv option is missing from benchmark/common.js #7890

adrian-nitu-92 opened this issue Jul 27, 2016 · 8 comments
Labels
benchmark Issues and PRs related to the benchmark subsystem. good first issue Issues that are suitable for first-time contributors.

Comments

@adrian-nitu-92
Copy link
Contributor

  • Version: v7.0.0-pre
  • Platform: Linux nova-node 4.2.0-27-generic # 32~14.04.1-Ubuntu SMP Fri Jan 22 15:32:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: benchmark

Patch f99471b
removes OUTPUT_FORMAT=csv option from benchmark/common.js. The patch seems to remove all OUTPUT_FORMAT options, with no alternative.
We found this option especially useful for automatically parsing benchmark results. Is there any chance for it to be re-introduced?
@ChALkeR ChALkeR added the benchmark Issues and PRs related to the benchmark subsystem. label Jul 27, 2016
@ChALkeR
Copy link
Member

ChALkeR commented Jul 27, 2016

/cc @nodejs/benchmarking

@addaleax
Copy link
Member

fwiw, running benchmark/scatter.js produces CSV output.

/cc @AndreasMadsen

@addaleax addaleax mentioned this issue Jul 27, 2016
Merged
4 tasks
@AndreasMadsen
Copy link
Member

AndreasMadsen commented Jul 27, 2016 •
edited
Loading

First of all, the script you should run is now benchmark/run.js, common.js is only used as require('common.js') in the benchmarks.

I don't think we should reintroduce this environment variable, it fells like an ad-hoc solution. But we could easily add a --csv flag to benchmark/run.js, that would do the same.

Would you like to do this yourself, or should I do it?

@AndreasMadsen AndreasMadsen added the good first issue Issues that are suitable for first-time contributors. label Jul 27, 2016
@mscdex
Copy link
Contributor

mscdex commented Jul 27, 2016

IMHO having a --format csv/--format=csv-style flag would be better

@AndreasMadsen
Copy link
Member

Agreed. It would be --format csv, that is how the other parameters works.

@adrian-nitu-92
Copy link
Contributor Author

I looked into the code and will soon send out a pull request that I will link here.

@adrian-nitu-92
Copy link
Contributor Author

Took a while, but I have a pull request available at: #7961

@AndreasMadsen
Copy link
Member

#7961 has now landed in 9e7fd8e 4b527a4 474e629.

@AndreasMadsen AndreasMadsen mentioned this issue Aug 26, 2016
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@mscdex @ChALkeR @AndreasMadsen @addaleax @adrian-nitu-92