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

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 <[email protected]>

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 <[email protected]>

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" [email protected] 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 d3482a0Compare October 29, 2015 02:50

bnoordhuis reviewed Oct 29, 2015
View reviewed changes

test/parallel/test-child-process-send-returns-boolean.js Outdated
Copy link
Member
bnoordhuisOct 29, 2015
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.

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 b904c2fCompare 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

Uh oh!

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

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

Uh oh!

Conversation

Trott commented Oct 28, 2015

Uh oh!

Trott commented Oct 29, 2015

Uh oh!

bnoordhuisOct 29, 2015

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented Oct 29, 2015

Uh oh!

Trott commented Oct 29, 2015

Uh oh!

bnoordhuis commented Oct 29, 2015

Uh oh!

Trott commented Oct 30, 2015

Uh oh!

rvagg commented Oct 30, 2015

Uh oh!

Trott commented Oct 30, 2015

Uh oh!

MylesBorins commented Dec 29, 2015

Uh oh!

Trott commented Dec 30, 2015

Uh oh!

rvagg commented Dec 30, 2015

Uh oh!

jasnell commented Dec 30, 2015

Uh oh!

MylesBorins commented Jan 6, 2016

Uh oh!

Trott commented Jan 7, 2016

Uh oh!

MylesBorins commented Jan 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants