Skip to content

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

Closed
Closed
OUTPUT_FORMAT=csv option is missing from benchmark/common.js#7890
Labels
benchmarkIssues and PRs related to Node.js benchmarks and benchmarking infrastructure.good first issueIssues that are suitable for first-time contributors.

Description

@adrian-nitu-92
adrian-nitu-92
opened on Jul 27, 2016
  • 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?

Activity

  1. ChALkeR
    added
    benchmarkIssues and PRs related to Node.js benchmarks and benchmarking infrastructure.
    on Jul 27, 2016

  2. ChALkeR commented on Jul 27, 2016

    @ChALkeR
    ChALkeR
    on Jul 27, 2016
    Member

    /cc @nodejs/benchmarking

  3. addaleax commented on Jul 27, 2016

    @addaleax
    addaleax
    on Jul 27, 2016
    Member

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

    /cc @AndreasMadsen

  4. AndreasMadsen commented on Jul 27, 2016

    @AndreasMadsen
    AndreasMadsen
    on Jul 27, 2016
    Member

    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?

  5. AndreasMadsen
    added
    good first issueIssues that are suitable for first-time contributors.
    on Jul 27, 2016

  6. mscdex commented on Jul 27, 2016

    @mscdex
    mscdex
    on Jul 27, 2016
    Contributor

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

  7. AndreasMadsen commented on Jul 27, 2016

    @AndreasMadsen
    AndreasMadsen
    on Jul 27, 2016
    Member

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

  8. adrian-nitu-92 commented on Jul 28, 2016

    @adrian-nitu-92
    adrian-nitu-92
    on Jul 28, 2016
    ContributorAuthor

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

  9. adrian-nitu-92 commented on Aug 3, 2016

    @adrian-nitu-92
    adrian-nitu-92
    on Aug 3, 2016
    ContributorAuthor

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

  10. AndreasMadsen
    closed this as completedin 474e629on Aug 12, 2016

  11. AndreasMadsen commented on Aug 12, 2016

    @AndreasMadsen
    AndreasMadsen
    on Aug 12, 2016
    Member

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

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    benchmarkIssues and PRs related to Node.js benchmarks and benchmarking infrastructure.good first issueIssues that are suitable for first-time contributors.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions