src: remove --expose-http2 option #20887

danbev · 2018-05-22T10:14:22Z

This commit removes the --expose-http2 option. In the code comment it
states that this should should be noop through v9.x, and it sounds like
it can be removed for 10.x and above.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

This commit removes the --expose-http2 option. In the code comment it states that this should should be noop through v9.x, and it sounds like it can be removed for 10.x and above.

danbev · 2018-05-22T10:14:52Z

CI: https://ci.nodejs.org/job/node-test-pull-request/15017/

addaleax

Still breaking if somebody uses the flag in 10.x and we remove it, right?

danbev · 2018-05-22T10:21:27Z

Still breaking if somebody uses the flag in 10.x and we remove it, right?

Oh yes, that is right. Thanks for adding the 10.x label!

targos · 2018-05-22T10:46:38Z

How about we put the semver-major label instead?

targos

There are also instances of the flag in the benchmark directory:

benchmark/http2/headers.js:}, { flags: ['--no-warnings', '--expose-http2'] });
benchmark/http2/respond-with-fd.js:}, { flags: ['--no-warnings', '--expose-http2'] });
benchmark/http2/simple.js:}, { flags: ['--no-warnings', '--expose-http2'] });
benchmark/http2/write.js:}, { flags: ['--no-warnings', '--expose-http2'] });

danbev · 2018-05-22T11:31:50Z

There are also instances of the flag in the benchmark directory

Thanks for that, will update shortly.

danbev · 2018-05-24T07:13:26Z

Landed in 3294d1b.

This commit removes the --expose-http2 option. In the code comment it states that this should should be noop through v9.x, and it sounds like it can be removed for 10.x and above. PR-URL: #20887 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Notable changes: * Build * FreeBSD 10 is no longer supported. [#22617](#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](#20442) * `crypto` * PEM-level encryption is now supported. [#23151](#23151) * An API for key pair generation has been added. [#22660](#22660) * Dependencies * V8 has been updated to 7.0. [#22754](#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](#20270) * `http2` * An event will be emitted when a `PING` frame is received. [#23009](#23009) * Support for the `ORIGIN` frame has been added. [#22956](#22956) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. * An experimental implementation of `queueMicrotask()` has been added. [#22951](#22951) * Internal * Windows performance-counter support has been removed. [#22485](#22485) * The `--expose-http2` command-line option has been removed. [#20887](#20887) * Promises * A new `multipleResolves` event will be emitted when a Promise is resolved (or rejected) more than once. [#22218](#22218) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](#21914)

Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](#20442) * Dependencies * V8 has been updated to 7.0. [#22754](#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](#22951) * Internal * Windows performance-counter support has been removed. [#22485](#22485) * The `--expose-http2` command-line option has been removed. [#20887](#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](#21914)

Notable changes: * Build * FreeBSD 10 is no longer supported.[nodejs#22617](nodejs#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [nodejs#21316](nodejs#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [nodejs#21649](nodejs#21649) * `console.time()` will no longer reset a timer if it already exists. [nodejs#20442](nodejs#20442) * Dependencies * V8 has been updated to 7.0. [nodejs#22754](nodejs#22754) * `fs` * The `fs.read()` method now requires a callback. [nodejs#22146](nodejs#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[nodejs#20735](nodejs#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [nodejs#20270](nodejs#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [nodejs#22951](nodejs#22951) * Internal * Windows performance-counter support has been removed. [nodejs#22485](nodejs#22485) * The `--expose-http2` command-line option has been removed. [nodejs#20887](nodejs#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [nodejs#20002](nodejs#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [nodejs#22281](nodejs#22281) * `util.inspect()` output size is limited to 128 MB by default. [nodejs#22756](nodejs#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [nodejs#21914](nodejs#21914)

Notable changes: * Build * FreeBSD 10 is no longer supported.[#22617](nodejs/node#22617) * `child_process` * The default value of the `windowsHide` option has been changed to `true`. [#21316](nodejs/node#21316) * `console` * `console.countReset()` will emit a warning if the timer being reset does not exist. [#21649](nodejs/node#21649) * `console.time()` will no longer reset a timer if it already exists. [#20442](nodejs/node#20442) * Dependencies * V8 has been updated to 7.0. [#22754](nodejs/node#22754) * `fs` * The `fs.read()` method now requires a callback. [#22146](nodejs/node#22146) * The previously deprecated `fs.SyncWriteStream` utility has been removed.[#20735](nodejs/node#20735) * `http` * The `http`, `https`, and `tls` modules now use the WHATWG URL parser by default. [#20270](nodejs/node#20270) * General * Use of `process.binding()` has been deprecated. Userland code using `process.binding()` should re-evaluate that use and begin migrating. If there are no supported API alternatives, please open an issue in the Node.js GitHub repository so that a suitable alternative may be discussed. * An experimental implementation of `queueMicrotask()` has been added. [#22951](nodejs/node#22951) * Internal * Windows performance-counter support has been removed. [#22485](nodejs/node#22485) * The `--expose-http2` command-line option has been removed. [#20887](nodejs/node#20887) * Timers * Interval timers will be rescheduled even if previous interval threw an error. [#20002](nodejs/node#20002) * `util` * The WHATWG `TextEncoder` and `TextDecoder` are now globals. [#22281](nodejs/node#22281) * `util.inspect()` output size is limited to 128 MB by default. [#22756](nodejs/node#22756) * A runtime warning will be emitted when `NODE_DEBUG` is set for either `http` or `http2`. [#21914](nodejs/node#21914)

src: remove --expose-http2 option

d588480

This commit removes the --expose-http2 option. In the code comment it states that this should should be noop through v9.x, and it sounds like it can be removed for 10.x and above.

danbev added dont-land-on-v4.x labels May 22, 2018

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 22, 2018

addaleax added the dont-land-on-v10.x label May 22, 2018

addaleax approved these changes May 22, 2018

View reviewed changes

targos requested changes May 22, 2018

View reviewed changes

apapirovski added semver-major PRs that contain breaking changes and should be released in the next major version. and removed dont-land-on-v10.x labels May 22, 2018

squash: remove --expose-http2 from benchmarks

2a3b629

targos approved these changes May 22, 2018

View reviewed changes

benjamingr approved these changes May 22, 2018

View reviewed changes

richardlau approved these changes May 22, 2018

View reviewed changes

cjihrig approved these changes May 22, 2018

View reviewed changes

trivikr approved these changes May 22, 2018

View reviewed changes

jasnell approved these changes May 22, 2018

View reviewed changes

danbev closed this May 24, 2018

danbev deleted the remove-expose-http2 branch May 24, 2018 07:13

jasnell mentioned this pull request Oct 2, 2018

11.0.0 proposal #23230

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: remove --expose-http2 option #20887

src: remove --expose-http2 option #20887

danbev commented May 22, 2018

danbev commented May 22, 2018

addaleax left a comment

danbev commented May 22, 2018

targos commented May 22, 2018

targos left a comment

danbev commented May 22, 2018

danbev commented May 24, 2018

src: remove --expose-http2 option #20887

src: remove --expose-http2 option #20887

Conversation

danbev commented May 22, 2018

Checklist

danbev commented May 22, 2018

addaleax left a comment

Choose a reason for hiding this comment

danbev commented May 22, 2018

targos commented May 22, 2018

targos left a comment

Choose a reason for hiding this comment

danbev commented May 22, 2018

danbev commented May 24, 2018