benchmark: chunky http client benchmark variation and server#228
benchmark: chunky http client benchmark variation and server#228rudi-cilibrasi wants to merge 3 commits intonodejs:v1.xfrom rudi-cilibrasi:benchmark-chunky-http
Conversation
benchmark/net/chunky_http_client.js
Outdated
There was a problem hiding this comment.
Style: please indent with two spaces here.
|
Thanks for the great comments Ben they are all done! |
There was a problem hiding this comment.
This could throw with Error: ENOENT. Instead do:
try {
fs.unlinkSync(PIPE);
} catch (e) { }
benchmark/net/chunky_http_client.js
Outdated
There was a problem hiding this comment.
Style: the convention is to use single quotes. There is usually a blank line after 'use strict'.
There was a problem hiding this comment.
If you're not going to handle them, you should let 'error' events bubble up.
|
Hi Trevor and Ben, I think the style is all set! Please let me know if you spot anything else I appreciate the practice. |
|
ping @trevnorris curious on your thoughts |
There was a problem hiding this comment.
Are you sure conf.len and conf.num will always be defined? If not they'll return NaN.
There was a problem hiding this comment.
Yes, this seems to be the assumption in all of the configuration code in the benchmark client headers net/ section. I am simply following that pre-existing convention. I did notice that there is a conf.len downstairs line 65 or so. I pushed (rebase -i) a minor cleanup fix for it to use 'len' instead. But both seem fine and correct to me so I doubt that this is more than a very minor aesthetic clarity issue.
|
One minor comment, but don't see anything I oppose here. |
|
I'm gonna bump this up and jump in. Hows this going? |
|
Fine. I'm happy to have this patch accepted when applicable. |
|
Seems you've addressed everything from Ben and myself. I'll LGTM. @bnoordhuis? |
|
ping @bnoordhuis it's been a couple weeks since anything has happened on this patch. may i ask if it is reasonable yet? |
|
@rudi-cilibrasi i'll give until monday for any other responses. if non come ping me and i'll land this. |
|
Sorry, the notification got lost in the gaping black hole that's my email backlog. I defer to @trevnorris; if he is happy with it, I'm happy with it. :-) |
|
no problems over the weekend. seems to be ready for acceptance. :-) On Fri, Mar 6, 2015 at 12:46 PM, Ben Noordhuis notifications@github.com
Happy to Code with Integrity : Software Engineering Code of Ethics and |
PR-URL: #228 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
Thanks. Landed in 030a923. |
Full release notes: A few meaty bugfixes, and introducing `peerDependenciesMeta`. FEATURES * [`a12341088`](npm/cli@a123410) [nodejs#224](npm/cli#224) Implements peerDependenciesMeta ([@arcanis](https://github.com/arcanis)) * [`2f3b79bba`](npm/cli@2f3b79b) [nodejs#234](npm/cli#234) add new forbidden 403 error code ([@claudiahdz](https://github.com/claudiahdz)) BUGFIXES * [`24acc9fc8`](npm/cli@24acc9f) and [`45772af0d`](npm/cli@45772af) [nodejs#217](npm/cli#217) [npm.community#8863](https://npm.community/t/installing-the-same-module-under-multiple-relative-paths-fails-on-linux/8863) [npm.community#9327](https://npm.community/t/reinstall-breaks-after-npm-update-to-6-10-2/9327,) do not descend into directory deps' child modules, fix shrinkwrap files that inappropriately list child nodes of symlink packages ([@isaacs](https://github.com/isaacs) and [@salomvary](https://github.com/salomvary)) * [`50cfe113d`](npm/cli@50cfe11) [nodejs#229](npm/cli#229) fixed typo in semver doc ([@gall0ws](https://github.com/gall0ws)) * [`e8fb2a1bd`](npm/cli@e8fb2a1) [nodejs#231](npm/cli#231) Fix spelling mistakes in CHANGELOG-3.md ([@XhmikosR](https://github.com/XhmikosR)) * [`769d2e057`](npm/cli@769d2e0) [npm/uid-number#7](npm/uid-number#7) Better error on invalid `--user`/`--group` configs. This addresses the issue when people fail to install binary packages on Docker and other environments where there is no 'nobody' user. ([@isaacs](https://github.com/isaacs)) * [`8b43c9624`](npm/cli@8b43c96) [nodejs#28987](nodejs#28987) [npm.community#6032](https://npm.community/t/npm-ci-doesnt-respect-npmrc-variables/6032) [npm.community#6658](https://npm.community/t/npm-ci-doesnt-fill-anymore-the-process-env-npm-config-cache-variable-on-post-install-scripts/6658) [npm.community#6069](https://npm.community/t/npm-ci-does-not-compile-native-dependencies-according-to-npmrc-configuration/6069) [npm.community#9323](https://npm.community/t/npm-6-9-x-not-passing-environment-to-node-gyp-regression-from-6-4-x/9323/2) Fix the regression where random config values in a .npmrc file are not passed to lifecycle scripts, breaking build processes which rely on them. ([@isaacs](https://github.com/isaacs)) * [`8b85eaa47`](npm/cli@8b85eaa) save files with inferred ownership rather than relying on `SUDO_UID` and `SUDO_GID`. ([@isaacs](https://github.com/isaacs)) * [`b7f6e5f02`](npm/cli@b7f6e5f) Infer ownership of shrinkwrap files ([@isaacs](https://github.com/isaacs)) * [`54b095d77`](npm/cli@54b095d) [nodejs#235](npm/cli#235) Add spec to dist-tag remove function ([@theberbie](https://github.com/theberbie)) DEPENDENCIES * [`dc8f9e52f`](npm/cli@dc8f9e5) `pacote@9.5.7`: Infer the ownership of all unpacked files in `node_modules`, so that we never have user-owned files in root-owned folders, or root-owned files in user-owned folders. ([@isaacs](https://github.com/isaacs)) * [`bb33940c3`](npm/cli@bb33940) `cmd-shim@3.0.0`: * [`9c93ac3`](npm/cmd-shim@9c93ac3) [#2](npm/cmd-shim#2) [npm#3380](npm/npm#3380) Handle environment variables properly ([@basbossink](https://github.com/basbossink)) * [`2d277f8`](npm/cmd-shim@2d277f8) [nodejs#25](npm/cmd-shim#25) [nodejs#36](npm/cmd-shim#36) [nodejs#35](npm/cmd-shim#35) Fix 'no shebang' case by always providing `$basedir` in shell script ([@igorklopov](https://github.com/igorklopov)) * [`adaf20b`](npm/cmd-shim@adaf20b) [nodejs#26](npm/cmd-shim#26) Fix `$*` causing an error when arguments contain parentheses ([@satazor](https://github.com/satazor)) * [`49f0c13`](npm/cmd-shim@49f0c13) [nodejs#30](npm/cmd-shim#30) Fix paths for MSYS/MINGW bash ([@dscho](https://github.com/dscho)) * [`51a8af3`](npm/cmd-shim@51a8af3) [nodejs#34](npm/cmd-shim#34) Add proper support for PowerShell ([@ExE-Boss](https://github.com/ExE-Boss)) * [`4c37e04`](npm/cmd-shim@4c37e04) [#10](npm/cmd-shim#10) Work around quoted batch file names ([@isaacs](https://github.com/isaacs)) * [`a4e279544`](npm/cli@a4e2795) `npm-lifecycle@3.1.3` ([@isaacs](https://github.com/isaacs)): * fail properly if `uid-number` raises an error * [`7086a1809`](npm/cli@7086a18) `libcipm@4.0.3` ([@isaacs](https://github.com/isaacs)) * [`8845141f9`](npm/cli@8845141) `read-package-json@2.1.0` ([@isaacs](https://github.com/isaacs)) * [`51c028215`](npm/cli@51c0282) `bin-links@1.1.3` ([@isaacs](https://github.com/isaacs)) * [`534a5548c`](npm/cli@534a554) `read-cmd-shim@1.0.3` ([@isaacs](https://github.com/isaacs)) * [`3038f2fd5`](npm/cli@3038f2f) `gentle-fs@2.2.1` ([@isaacs](https://github.com/isaacs)) * [`a609a1648`](npm/cli@a609a16) `graceful-fs@4.2.2` ([@isaacs](https://github.com/isaacs)) * [`f0346f754`](npm/cli@f0346f7) `cacache@12.0.3` ([@isaacs](https://github.com/isaacs)) * [`ca9c615c8`](npm/cli@ca9c615) `npm-pick-manifest@3.0.0` ([@isaacs](https://github.com/isaacs)) * [`b417affbf`](npm/cli@b417aff) `pacote@9.5.8` ([@isaacs](https://github.com/isaacs)) TESTS * [`b6df0913c`](npm/cli@b6df091) [nodejs#228](npm/cli#228) Proper handing of /usr/bin/node lifecycle-path test ([@olivr70](https://github.com/olivr70)) * [`aaf98e88c`](npm/cli@aaf98e8) `npm-registry-mock@1.3.0` ([@isaacs](https://github.com/isaacs))
Full changelog: 6.11.1 (2019-08-20): Fix a regression for windows command shim syntax. * [`37db29647`](npm/cli@37db296) `cmd-shim@3.0.2` ([@isaacs](https://github.com/isaacs)) v6.11.0 (2019-08-20): A few meaty bugfixes, and introducing `peerDependenciesMeta`. FEATURES * [`a12341088`](npm/cli@a123410) [nodejs#224](npm/cli#224) Implements peerDependenciesMeta ([@arcanis](https://github.com/arcanis)) * [`2f3b79bba`](npm/cli@2f3b79b) [nodejs#234](npm/cli#234) add new forbidden 403 error code ([@claudiahdz](https://github.com/claudiahdz)) BUGFIXES * [`24acc9fc8`](npm/cli@24acc9f) and [`45772af0d`](npm/cli@45772af) [nodejs#217](npm/cli#217) [npm.community#8863](https://npm.community/t/installing-the-same-module-under-multiple-relative-paths-fails-on-linux/8863) [npm.community#9327](https://npm.community/t/reinstall-breaks-after-npm-update-to-6-10-2/9327,) do not descend into directory deps' child modules, fix shrinkwrap files that inappropriately list child nodes of symlink packages ([@isaacs](https://github.com/isaacs) and [@salomvary](https://github.com/salomvary)) * [`50cfe113d`](npm/cli@50cfe11) [nodejs#229](npm/cli#229) fixed typo in semver doc ([@gall0ws](https://github.com/gall0ws)) * [`e8fb2a1bd`](npm/cli@e8fb2a1) [nodejs#231](npm/cli#231) Fix spelling mistakes in CHANGELOG-3.md ([@XhmikosR](https://github.com/XhmikosR)) * [`769d2e057`](npm/cli@769d2e0) [npm/uid-number#7](npm/uid-number#7) Better error on invalid `--user`/`--group` configs. This addresses the issue when people fail to install binary packages on Docker and other environments where there is no 'nobody' user. ([@isaacs](https://github.com/isaacs)) * [`8b43c9624`](npm/cli@8b43c96) [nodejs#28987](nodejs#28987) [npm.community#6032](https://npm.community/t/npm-ci-doesnt-respect-npmrc-variables/6032) [npm.community#6658](https://npm.community/t/npm-ci-doesnt-fill-anymore-the-process-env-npm-config-cache-variable-on-post-install-scripts/6658) [npm.community#6069](https://npm.community/t/npm-ci-does-not-compile-native-dependencies-according-to-npmrc-configuration/6069) [npm.community#9323](https://npm.community/t/npm-6-9-x-not-passing-environment-to-node-gyp-regression-from-6-4-x/9323/2) Fix the regression where random config values in a .npmrc file are not passed to lifecycle scripts, breaking build processes which rely on them. ([@isaacs](https://github.com/isaacs)) * [`8b85eaa47`](npm/cli@8b85eaa) save files with inferred ownership rather than relying on `SUDO_UID` and `SUDO_GID`. ([@isaacs](https://github.com/isaacs)) * [`b7f6e5f02`](npm/cli@b7f6e5f) Infer ownership of shrinkwrap files ([@isaacs](https://github.com/isaacs)) * [`54b095d77`](npm/cli@54b095d) [nodejs#235](npm/cli#235) Add spec to dist-tag remove function ([@theberbie](https://github.com/theberbie)) DEPENDENCIES * [`dc8f9e52f`](npm/cli@dc8f9e5) `pacote@9.5.7`: Infer the ownership of all unpacked files in `node_modules`, so that we never have user-owned files in root-owned folders, or root-owned files in user-owned folders. ([@isaacs](https://github.com/isaacs)) * [`bb33940c3`](npm/cli@bb33940) `cmd-shim@3.0.0`: * [`9c93ac3`](npm/cmd-shim@9c93ac3) [#2](npm/cmd-shim#2) [npm#3380](npm/npm#3380) Handle environment variables properly ([@basbossink](https://github.com/basbossink)) * [`2d277f8`](npm/cmd-shim@2d277f8) [nodejs#25](npm/cmd-shim#25) [nodejs#36](npm/cmd-shim#36) [nodejs#35](npm/cmd-shim#35) Fix 'no shebang' case by always providing `$basedir` in shell script ([@igorklopov](https://github.com/igorklopov)) * [`adaf20b`](npm/cmd-shim@adaf20b) [nodejs#26](npm/cmd-shim#26) Fix `$*` causing an error when arguments contain parentheses ([@satazor](https://github.com/satazor)) * [`49f0c13`](npm/cmd-shim@49f0c13) [nodejs#30](npm/cmd-shim#30) Fix paths for MSYS/MINGW bash ([@dscho](https://github.com/dscho)) * [`51a8af3`](npm/cmd-shim@51a8af3) [nodejs#34](npm/cmd-shim#34) Add proper support for PowerShell ([@ExE-Boss](https://github.com/ExE-Boss)) * [`4c37e04`](npm/cmd-shim@4c37e04) [#10](npm/cmd-shim#10) Work around quoted batch file names ([@isaacs](https://github.com/isaacs)) * [`a4e279544`](npm/cli@a4e2795) `npm-lifecycle@3.1.3` ([@isaacs](https://github.com/isaacs)): * fail properly if `uid-number` raises an error * [`7086a1809`](npm/cli@7086a18) `libcipm@4.0.3` ([@isaacs](https://github.com/isaacs)) * [`8845141f9`](npm/cli@8845141) `read-package-json@2.1.0` ([@isaacs](https://github.com/isaacs)) * [`51c028215`](npm/cli@51c0282) `bin-links@1.1.3` ([@isaacs](https://github.com/isaacs)) * [`534a5548c`](npm/cli@534a554) `read-cmd-shim@1.0.3` ([@isaacs](https://github.com/isaacs)) * [`3038f2fd5`](npm/cli@3038f2f) `gentle-fs@2.2.1` ([@isaacs](https://github.com/isaacs)) * [`a609a1648`](npm/cli@a609a16) `graceful-fs@4.2.2` ([@isaacs](https://github.com/isaacs)) * [`f0346f754`](npm/cli@f0346f7) `cacache@12.0.3` ([@isaacs](https://github.com/isaacs)) * [`ca9c615c8`](npm/cli@ca9c615) `npm-pick-manifest@3.0.0` ([@isaacs](https://github.com/isaacs)) * [`b417affbf`](npm/cli@b417aff) `pacote@9.5.8` ([@isaacs](https://github.com/isaacs)) TESTS * [`b6df0913c`](npm/cli@b6df091) [nodejs#228](npm/cli#228) Proper handing of /usr/bin/node lifecycle-path test ([@olivr70](https://github.com/olivr70)) * [`aaf98e88c`](npm/cli@aaf98e8) `npm-registry-mock@1.3.0` ([@isaacs](https://github.com/isaacs))
No description provided.