lib: return boolean from child.send() #3577

Trott · 2015-10-28T23:43:39Z

Previous change reinstated returning boolean from child.send() but
missed one instance where undefined might be returned instead.

R=@bnoordhuis

Trott · 2015-10-29T01:17:48Z

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

bnoordhuis · 2015-10-29T11:58:10Z

test/parallel/test-child-process-send-returns-boolean.js


 const rv = n.send({ hello: 'world' });
 assert.strictEqual(rv, true);
+
+const s = spawn(process.execPath, [emptyFile],
+  { stdio: ['pipe', 'pipe', 'pipe', 'ipc'] });


Nit: argument should line up.

bnoordhuis · 2015-10-29T11:59:55Z

LGTM with style nit.

Previous change reinstated returning boolean from child.send() but missed one instance where undefined might be returned instead.

Trott · 2015-10-29T12:29:51Z

Re-running CI after fixing up style nit: https://ci.nodejs.org/job/node-test-pull-request/646/

bnoordhuis · 2015-10-29T12:49:15Z

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.

Trott · 2015-10-30T00:01:32Z

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>

rvagg · 2015-10-30T08:19:42Z

@Trott I'm thinking this could be semver-minor, whaddya reckon?

Trott · 2015-10-30T15:18:05Z

@rvagg I personally wouldn't think so, but if there's any doubt, then yeah, let's be conservative and go with semver-minor.

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

MylesBorins · 2015-12-29T21:48:25Z

I just want to confirm that this is 100% being considered a patch before merging into LTS

/cc @rvagg @jasnell @Trott

Trott · 2015-12-30T06:15:02Z

@thealphanerd I consider it patch-level. But I wouldn't put up any resistance if someone else felt differently.

rvagg · 2015-12-30T12:20:52Z

+1 for LTS @thealphanerd

jasnell · 2015-12-30T14:21:49Z

+1 for LTS
On Dec 30, 2015 4:21 AM, "Rod Vagg" notifications@github.com wrote:

+1 for LTS @thealphanerd https://github.com/TheAlphaNerd

—
Reply to this email directly or view it on GitHub
#3577 (comment).

MylesBorins · 2016-01-06T23:40:04Z

So I just attempted to merge this into LTS and got conflicts as we have not landed #3516

As that is not landing does it make sense to add this change?

/cc @Trott @jasnell @rvagg

Trott · 2016-01-07T03:31:52Z

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?

MylesBorins · 2016-01-07T21:20:28Z

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

Trott added the child_process Issues and PRs related to the child_process subsystem. label Oct 28, 2015

Trott changed the title ~~lib,doc: return boolean from child.send()~~ lib: return boolean from child.send() Oct 29, 2015

Trott force-pushed the child-send-again branch from 6ad9583 to d3482a0 Compare October 29, 2015 02:50

bnoordhuis reviewed Oct 29, 2015
View reviewed changes

Trott added 2 commits October 29, 2015 05:26

lib: return boolean from child.send()

f24c29f

Previous change reinstated returning boolean from child.send() but missed one instance where undefined might be returned instead.

fixup

b904c2f

Trott force-pushed the child-send-again branch from d3482a0 to b904c2f Compare October 29, 2015 12:29

Trott closed this Oct 30, 2015

Fishrock123 mentioned this pull request Nov 6, 2015

Release proposal: v5.1.0 #3700

Closed

Fishrock123 mentioned this pull request Nov 11, 2015

Propose v5.1.0 #3736

Merged

rvagg mentioned this pull request Dec 17, 2015

Release 4.2.4 Planning #4321

Closed

jasnell added the lts-watch-v4.x label Dec 17, 2015

MylesBorins removed the lts-watch-v4.x label Jan 7, 2016

MylesBorins mentioned this pull request Jan 14, 2016

Audit commits not found on v4.x-staging #4698

Closed

rvagg added the dont-land-on-v4.x label Jan 15, 2016

mscdex mentioned this pull request Jul 14, 2016

Child.send boolean #7739

Closed

4 tasks

Trott deleted the child-send-again branch January 13, 2022 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: return boolean from child.send() #3577

lib: return boolean from child.send() #3577

Trott commented Oct 28, 2015

Trott commented Oct 29, 2015

bnoordhuis Oct 29, 2015

bnoordhuis commented Oct 29, 2015

Trott commented Oct 29, 2015

bnoordhuis commented Oct 29, 2015

Trott commented Oct 30, 2015

rvagg commented Oct 30, 2015

Trott commented Oct 30, 2015

MylesBorins commented Dec 29, 2015

Trott commented Dec 30, 2015

rvagg commented Dec 30, 2015

jasnell commented Dec 30, 2015

MylesBorins commented Jan 6, 2016

Trott commented Jan 7, 2016

MylesBorins commented Jan 7, 2016

lib: return boolean from child.send() #3577

lib: return boolean from child.send() #3577

Conversation

Trott commented Oct 28, 2015

Trott commented Oct 29, 2015

bnoordhuis Oct 29, 2015

Choose a reason for hiding this comment

bnoordhuis commented Oct 29, 2015

Trott commented Oct 29, 2015

bnoordhuis commented Oct 29, 2015

Trott commented Oct 30, 2015

rvagg commented Oct 30, 2015

Trott commented Oct 30, 2015

MylesBorins commented Dec 29, 2015

Trott commented Dec 30, 2015

rvagg commented Dec 30, 2015

jasnell commented Dec 30, 2015

MylesBorins commented Jan 6, 2016

Trott commented Jan 7, 2016

MylesBorins commented Jan 7, 2016