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

Removing the bench-* targets in makefile? #17053

Closed
joyeecheung opened this issue Nov 15, 2017 · 4 comments
Closed

Removing the bench-* targets in makefile? #17053

joyeecheung opened this issue Nov 15, 2017 · 4 comments
Labels
benchmark Issues and PRs related to the benchmark subsystem. build Issues and PRs related to build files or the CI. discuss Issues opened for discussions and feedbacks.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Nov 15, 2017 •
edited
Loading

  • Version: master
  • Subsystem: build, benchmark

Hi all, just wondering is there anyone actually using the bench-* targets in the Makefile? This is not used by the benchmark CI, and basically those just call ./node benchmark/run.js * which is not really that hard to remember (and this does not even allow you to set the arguments at all. Besides, these days we tend to use benchmark/compare.js instead). The bench-all and bench target somehow only run a subset of the benchmarks and they are not covering all the categories that we have anyway. Based on this I think those are not maintained and not really used anymore so I am just wondering maybe we can remove those altogether.

Refs: #16934
@joyeecheung joyeecheung added build Issues and PRs related to build files or the CI. benchmark Issues and PRs related to the benchmark subsystem. labels Nov 15, 2017
@joyeecheung
Copy link
Member Author

joyeecheung commented Nov 15, 2017 •
edited
Loading

cc @nodejs/build @nodejs/performance @nodejs/benchmarking

@gibfahn
Copy link
Member

gibfahn commented Nov 15, 2017

SGTM

@gareth-ellis
Copy link
Member

SGTM too,

@rvagg
Copy link
Member

rvagg commented Nov 15, 2017

not aware of anyone else using it, so sgtm and +1 for decluttering Makefile in general

@maclover7 maclover7 added the discuss Issues opened for discussions and feedbacks. label Dec 25, 2017
@joyeecheung joyeecheung mentioned this issue Jan 3, 2018
Closed
4 tasks
@joyeecheung joyeecheung mentioned this issue Jan 14, 2018
Closed
4 tasks
evanlucas pushed a commit that referenced this issue Jan 30, 2018
@joyeecheung @evanlucas
build: remove bench-* targets
1506eb5 
PR-URL: #18150
Fixes: #17053
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@richardlau richardlau mentioned this issue May 23, 2018
Closed
2 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. build Issues and PRs related to build files or the CI. discuss Issues opened for discussions and feedbacks.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: remove bench-* targets joyeecheung/node
5 participants
@rvagg @maclover7 @joyeecheung @gareth-ellis @gibfahn