-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
build: remove bench-* targets #18150
Conversation
bench: bench-net bench-http bench-fs bench-tls | ||
|
||
.PHONY: bench-ci | ||
bench-ci: bench |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like bench-ci
got completely removed -- should it also get the "deprecation warning"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI is not using bench-ci so I think the chance of anyone using it is much smaller...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but can you add a quick comment to the commit explaining why? :-)
@jasnell Sure, something like:
This can be fixed during landing. |
Landed in 0c8aaf3, thanks! |
@joyeecheung You left in the Is it possible that the benchmark is quietly broken? It exits without error when the binding.node file is missing. |
@bnoordhuis No I think I just forgot about |
This comment has been minimized.
This comment has been minimized.
@joyeecheung Should this be backported to |
Fixes: #17053
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
build