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

benchmark: add info about required Unix tools #8788

Closed
wants to merge 2 commits into from

Conversation

bzoz
Copy link
Contributor

@bzoz bzoz commented Sep 26, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmark

Description of change

This adds note to README.md about Unix tools being required by some benchmarks as requested in #8721 discussion.

/cc @nodejs/benchmarking

This adds note to README.md about Unix tools being required by some benchmarks
@bzoz bzoz added the benchmark Issues and PRs related to the benchmark subsystem. label Sep 26, 2016
@nodejs-github-bot nodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Sep 26, 2016
@mscdex mscdex added the doc Issues and PRs related to the documentations. label Sep 26, 2016
@@ -34,6 +34,10 @@ benchmarker to be used by providing it as an argument, e. g.:

`node benchmark/http/simple.js benchmarker=autocannon`

Basic Unix tools are required for some benchmarks.
[Git for Windows][git-for-windows] includes Git Bash and tools which can be
included in the global `PATH`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Git for Windows includes Git bash and the necessary tools, which need to be included in the global Windows path

@bzoz
Copy link
Contributor Author

bzoz commented Sep 27, 2016

Updated, PTAL

@lpinca
Copy link
Member

lpinca commented Sep 27, 2016

LGTM

@gibfahn
Copy link
Member

gibfahn commented Sep 27, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Sep 27, 2016

LGTM

1 similar comment
@mhdawson
Copy link
Member

LGTM

jasnell pushed a commit that referenced this pull request Sep 30, 2016
This adds note to README.md about Unix tools being
required by some benchmarks

PR-URL: #8788
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@jasnell
Copy link
Member

jasnell commented Sep 30, 2016

Landed in 3e7da1d. thank you!

@jasnell jasnell closed this Sep 30, 2016
jasnell pushed a commit that referenced this pull request Oct 6, 2016
This adds note to README.md about Unix tools being
required by some benchmarks

PR-URL: #8788
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@Fishrock123
Copy link
Member

Conflicts and isn't really necessary on v6, feel free to make a PR if you really want it.

@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants