-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
lib: return boolean from child.send() #3577
Conversation
6ad9583
to
d3482a0
Compare
|
||
const rv = n.send({ hello: 'world' }); | ||
assert.strictEqual(rv, true); | ||
|
||
const s = spawn(process.execPath, [emptyFile], | ||
{ stdio: ['pipe', 'pipe', 'pipe', 'ipc'] }); |
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.
Nit: argument should line up.
LGTM with style nit. |
Previous change reinstated returning boolean from child.send() but missed one instance where undefined might be returned instead.
d3482a0
to
b904c2f
Compare
Re-running CI after fixing up style nit: https://ci.nodejs.org/job/node-test-pull-request/646/ |
I suppose you can't be too careful but I don't think anyone would yell at you if you added the spaces and landed it. |
Landed in cf0130d |
Previous change reinstated returning boolean from child.send() but missed one instance where undefined might be returned instead. PR-URL: #3577 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@Trott I'm thinking this could be semver-minor, whaddya reckon? |
@rvagg I personally wouldn't think so, but if there's any doubt, then yeah, let's be conservative and go with |
Previous change reinstated returning boolean from child.send() but missed one instance where undefined might be returned instead. PR-URL: #3577 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Notable changes: * **child_process**: child.send() now properly returns a boolean like the docs suggest (Rich Trott) nodejs#3577. * **http_parser**: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * **npm**: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * **repl**: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs#3630.
Notable changes: * **child_process**: child.send() now properly returns a boolean like the docs suggest (Rich Trott) nodejs#3577. * **http_parser**: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * **npm**: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * **repl**: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs#3630. PR-URL: nodejs#3736
Notable changes: * buffer: The `noAssert` option for many buffer functions will now silently drop invalid write values rather than crashing (P.S.V.R) #3767. - This makes the behavior match what the docs suggest. * child_process: `child.send()` now properly returns a boolean like the docs suggest (Rich Trott) #3577. * doc: All of the API docs have been re-ordered so as to read in alphabetical order (Tristian Flanagan) #3662. * http_parser: update http-parser to 2.6.0 from 2.5.0 (James M Snell) #3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * npm: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) #3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * repl: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) #3630. * tls: The default `sessionIdContext` now uses SHA1 in FIPS mode rather than MD5 (Stefan Budeanu) #3755. * v8: Added some more useful post-mortem data (Fedor Indutny) #3779. PR-URL: #3736
@thealphanerd I consider it patch-level. But I wouldn't put up any resistance if someone else felt differently. |
+1 for LTS @thealphanerd |
+1 for LTS
|
This is basically a patch-fix to a previous semver minor, I guess it makes sense to not land it unless you are landing the semver minor patch first. Does that make sense? |
That was my thoughts exactly. I am removing the lts-watch label. Please feel free to audit and change this @jasnell |
Notable changes: * buffer: The `noAssert` option for many buffer functions will now silently drop invalid write values rather than crashing (P.S.V.R) nodejs/node#3767. - This makes the behavior match what the docs suggest. * child_process: `child.send()` now properly returns a boolean like the docs suggest (Rich Trott) nodejs/node#3577. * doc: All of the API docs have been re-ordered so as to read in alphabetical order (Tristian Flanagan) nodejs/node#3662. * http_parser: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs/node#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * npm: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs/node#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * repl: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs/node#3630. * tls: The default `sessionIdContext` now uses SHA1 in FIPS mode rather than MD5 (Stefan Budeanu) nodejs/node#3755. * v8: Added some more useful post-mortem data (Fedor Indutny) nodejs/node#3779. PR-URL: nodejs/node#3736
Notable changes: * buffer: The `noAssert` option for many buffer functions will now silently drop invalid write values rather than crashing (P.S.V.R) nodejs/node#3767. - This makes the behavior match what the docs suggest. * child_process: `child.send()` now properly returns a boolean like the docs suggest (Rich Trott) nodejs/node#3577. * doc: All of the API docs have been re-ordered so as to read in alphabetical order (Tristian Flanagan) nodejs/node#3662. * http_parser: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs/node#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * npm: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs/node#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * repl: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs/node#3630. * tls: The default `sessionIdContext` now uses SHA1 in FIPS mode rather than MD5 (Stefan Budeanu) nodejs/node#3755. * v8: Added some more useful post-mortem data (Fedor Indutny) nodejs/node#3779. PR-URL: nodejs/node#3736
Notable changes: * buffer: The `noAssert` option for many buffer functions will now silently drop invalid write values rather than crashing (P.S.V.R) nodejs/node#3767. - This makes the behavior match what the docs suggest. * child_process: `child.send()` now properly returns a boolean like the docs suggest (Rich Trott) nodejs/node#3577. * doc: All of the API docs have been re-ordered so as to read in alphabetical order (Tristian Flanagan) nodejs/node#3662. * http_parser: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs/node#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * npm: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs/node#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * repl: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs/node#3630. * tls: The default `sessionIdContext` now uses SHA1 in FIPS mode rather than MD5 (Stefan Budeanu) nodejs/node#3755. * v8: Added some more useful post-mortem data (Fedor Indutny) nodejs/node#3779. PR-URL: nodejs/node#3736
Notable changes: * buffer: The `noAssert` option for many buffer functions will now silently drop invalid write values rather than crashing (P.S.V.R) nodejs/node#3767. - This makes the behavior match what the docs suggest. * child_process: `child.send()` now properly returns a boolean like the docs suggest (Rich Trott) nodejs/node#3577. * doc: All of the API docs have been re-ordered so as to read in alphabetical order (Tristian Flanagan) nodejs/node#3662. * http_parser: update http-parser to 2.6.0 from 2.5.0 (James M Snell) nodejs/node#3569. - Now supports the following HTTP methods: `LINK`, `UNLINK`, `BIND`, `REBIND`, `UNBIND`. - Also added ACL and IPv6 Zone ID support. * npm: upgrade npm to 3.3.12 from v3.3.6 (Rebecca Turner) nodejs/node#3685. - See the release notes for https://github.com/npm/npm/releases/tag/v3.3.7, https://github.com/npm/npm/releases/tag/v3.3.8, https://github.com/npm/npm/releases/tag/v3.3.9, https://github.com/npm/npm/releases/tag/v3.3.10, https://github.com/npm/npm/releases/tag/v3.3.11, and https://github.com/npm/npm/releases/tag/v3.3.12 for more details. * repl: The REPL no longer crashes if the persistent history file cannot be opened (Evan Lucas) nodejs/node#3630. * tls: The default `sessionIdContext` now uses SHA1 in FIPS mode rather than MD5 (Stefan Budeanu) nodejs/node#3755. * v8: Added some more useful post-mortem data (Fedor Indutny) nodejs/node#3779. PR-URL: nodejs/node#3736
Previous change reinstated returning boolean from child.send() but
missed one instance where undefined might be returned instead.
R=@bnoordhuis