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

http,https,net,tls: return this from setTimeout methods #1699

Closed
wants to merge 1 commit into from

Conversation

evanlucas
Copy link
Contributor

Modifies the setTimeout methods for the following prototypes:

  • http.ClientRequest
  • http.IncomingMessage
  • http.OutgoingMessage
  • http.Server
  • https.Server
  • net.Socket
  • tls.TLSSocket

Previously, the above functions returned undefined. They now return
this. This is useful for chaining function calls.

Related: #1657

@evanlucas evanlucas added http Issues or PRs related to the http subsystem. net Issues and PRs related to the net subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels May 14, 2015
@brendanashworth
Copy link
Contributor

Why is this semver-major?

@silverwind
Copy link
Contributor

Going from undefined to something looks more like a semver-minor to me. Also, I think https needs the same treatment?

@evanlucas evanlucas removed the semver-major PRs that contain breaking changes and should be released in the next major version. label May 14, 2015
@evanlucas
Copy link
Contributor Author

Yea, I would think changing the return value of a function would be breaking regardless, but either way is fine by me.

@silverwind silverwind added the semver-minor PRs that contain new features and should be released in the next minor version. label May 14, 2015
Modifies the setTimeout methods for the following prototypes:

- http.ClientRequest
- http.IncomingMessage
- http.OutgoingMessage
- http.Server
- https.Server
- net.Socket
- tls.TLSSocket

Previously, the above functions returned undefined. They now return
`this`. This is useful for chaining function calls.
@evanlucas evanlucas changed the title http,net: return this from setTimeout methods http,https,net,tls: return this from setTimeout methods May 14, 2015
@evanlucas
Copy link
Contributor Author

Ok, updated tests to verify for https.Server and tls.TLSSocket as well

@silverwind
Copy link
Contributor

LGTM.

Let's land this one now, the other methods mentioned in #1657 can follow later. I think it's safe to say that we don't ever want to return anything else on the affected methods.

@evanlucas
Copy link
Contributor Author

@silverwind
Copy link
Contributor

Looks like the usual unrelated windows and arm failures. I think it's good to merge.

evanlucas added a commit that referenced this pull request May 16, 2015
Modifies the setTimeout methods for the following prototypes:

- http.ClientRequest
- http.IncomingMessage
- http.OutgoingMessage
- http.Server
- https.Server
- net.Socket
- tls.TLSSocket

Previously, the above functions returned undefined. They now return
`this`. This is useful for chaining function calls.

PR-URL: #1699
Reviewed-By: Roman Reiss <[email protected]>
@silverwind
Copy link
Contributor

Landed in d4726cd with a slightly shorter commit title (ommited https to fit)

@silverwind silverwind closed this May 16, 2015
rvagg added a commit to rvagg/io.js that referenced this pull request May 23, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit to rvagg/io.js that referenced this pull request May 23, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit to rvagg/io.js that referenced this pull request May 24, 2015
PR-URL: nodejs#1532

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) nodejs#1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) nodejs#1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) nodejs#1749.
rvagg added a commit that referenced this pull request May 24, 2015
PR-URL: #1777

Notable Changes:

* crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be
  1024 bits or longer or an error will be thrown. A warning will also be printed
  to the console if you supply less than 2048 bits. See https://weakdh.org/ for
  further context on this security concern. (Shigeki Ohtsu) #1739.
* node: A new --trace-sync-io command line flag will print a warning and a stack
  trace whenever a synchronous API is used. This can be used to track down
  synchronous calls that may be slowing down an application.
  (Trevor Norris) #1707.
* node: To allow for chaining of methods, the setTimeout(), setKeepAlive(),
  setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http',
  'https' and 'tls' now return the current instance instead of undefined
  (Roman Reiss & Evan Lucas) #1699 #1768 #1779.
* npm: Upgraded to v2.10.1, release notes can be found in
  https://github.com/npm/npm/releases/tag/v2.10.1 and
  https://github.com/npm/npm/releases/tag/v2.10.0.
* util: A significant speed-up (in the order of 35%) for the common-case of a
  single string argument to util.format(), used by console.log()
  (Сковорода Никита Андреевич) #1749.
andrewdeandrade pushed a commit to andrewdeandrade/node that referenced this pull request Jun 3, 2015
Modifies the setTimeout methods for the following prototypes:

- http.ClientRequest
- http.IncomingMessage
- http.OutgoingMessage
- http.Server
- https.Server
- net.Socket
- tls.TLSSocket

Previously, the above functions returned undefined. They now return
`this`. This is useful for chaining function calls.

PR-URL: nodejs/node#1699
Reviewed-By: Roman Reiss <[email protected]>
MikeRalphson added a commit to MikeRalphson/node that referenced this pull request Aug 31, 2016
MikeRalphson added a commit to MikeRalphson/node that referenced this pull request Aug 31, 2016
MikeRalphson added a commit to MikeRalphson/node that referenced this pull request Aug 31, 2016
lpinca pushed a commit to lpinca/node that referenced this pull request Sep 3, 2016
Refs: nodejs#1699
PR-URL: nodejs#8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
Refs: nodejs#1699
PR-URL: nodejs#8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
Refs: #1699
PR-URL: #8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 30, 2016
Refs: #1699
PR-URL: #8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2016
Refs: #1699
PR-URL: #8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Refs: #1699
PR-URL: #8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Refs: #1699
PR-URL: #8356
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem. net Issues and PRs related to the net subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants