FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: return boolean from child.send() by Trott · Pull Request #3577 · nodejs/node · GitHub

/ node Public

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

Closed
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:child-send-again
ConversationCommits (2)ChecksFiles changed
Closed

lib: return boolean from child.send()#3577
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:child-send-again

Conversation

Trott commented Oct 28, 2015

Copy link
Copy Markdown
Member

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

R=@bnoordhuis

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

Trott commented Oct 29, 2015

Copy link
Copy Markdown
Member Author

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

Trott changed the title lib,doc: return boolean from child.send() lib: return boolean from child.send() Oct 29, 2015
bnoordhuis reviewed Oct 29, 2015
View reviewed changes
Comment thread test/parallel/test-child-process-send-returns-boolean.js Outdated

bnoordhuis Oct 29, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nit: argument should line up.

bnoordhuis commented Oct 29, 2015

Copy link
Copy Markdown
Member

LGTM with style nit.

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 commented Oct 29, 2015

Copy link
Copy Markdown
Member Author

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

bnoordhuis commented Oct 29, 2015

Copy link
Copy Markdown
Member

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 commented Oct 30, 2015

Copy link
Copy Markdown
Member Author

Landed in cf0130d

Trott closed this Oct 30, 2015
Trott added a commit that referenced this pull request Oct 30, 2015
lib: return boolean from child.send()
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 commented Oct 30, 2015

Copy link
Copy Markdown
Member

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

Trott commented Oct 30, 2015

Copy link
Copy Markdown
Member Author

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

Fishrock123 mentioned this pull request Nov 6, 2015
Closed
Trott added a commit that referenced this pull request Nov 7, 2015
lib: return boolean from child.send()
d2b5dcb 
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>
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Nov 10, 2015
2015-11-10, Version 5.1.0 (Stable)
f609dd2 
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.
Fishrock123 mentioned this pull request Nov 11, 2015
Merged
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Nov 17, 2015
2015-11-17, Version 5.1.0 (Stable)
335077f 
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
Fishrock123 added a commit that referenced this pull request Nov 17, 2015
2015-11-17, Version 5.1.0 (Stable)
02fd81a 
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
rvagg mentioned this pull request Dec 17, 2015
Closed

MylesBorins commented Dec 29, 2015

Copy link
Copy Markdown
Contributor

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

/cc @rvagg @jasnell @Trott

Trott commented Dec 30, 2015

Copy link
Copy Markdown
Member Author

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

rvagg commented Dec 30, 2015

Copy link
Copy Markdown
Member

+1 for LTS @thealphanerd

jasnell commented Dec 30, 2015

Copy link
Copy Markdown
Member

+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 commented Jan 6, 2016

Copy link
Copy Markdown
Contributor

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 commented Jan 7, 2016

Copy link
Copy Markdown
Member Author

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 commented Jan 7, 2016

Copy link
Copy Markdown
Contributor

That was my thoughts exactly. I am removing the lts-watch label. Please feel free to audit and change this @jasnell

MylesBorins mentioned this pull request Jan 14, 2016
Closed
bbondy pushed a commit to brave/node that referenced this pull request Mar 13, 2016
2015-11-17, Version 5.1.0 (Stable)
f3c29fc 
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
bbondy pushed a commit to brave/node that referenced this pull request Mar 14, 2016
2015-11-17, Version 5.1.0 (Stable)
d76b19c 
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
bbondy pushed a commit to brave/node that referenced this pull request Mar 14, 2016
2015-11-17, Version 5.1.0 (Stable)
22187ec 
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
bbondy pushed a commit to brave/node that referenced this pull request Mar 15, 2016
2015-11-17, Version 5.1.0 (Stable)
fbd3d02 
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
mscdex mentioned this pull request Jul 14, 2016
Closed
4 tasks
Trott deleted the child-send-again branch January 13, 2022 22:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

child_process Issues and PRs related to the child_process subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL