doc: refactor signal info in child_process.md - #37528
Conversation
Sorry, something went wrong.
|
Maybe we should add the info in the YAML list as well: - version: v15.4.0
pr-url: https://github.com/nodejs/node/pull/36308
description: AbortSignal support was added. |
Sorry, something went wrong.
There was a problem hiding this comment.
An example would be nice here :]
Sorry, something went wrong.
There was a problem hiding this comment.
I actually think examples in APIs are better than not - since users often don't know all the APIs they're working with - sending them to another API (spawn) when they're trying to use execFile isn't better than an example using execFile IMO.
The fact internally execFile delegates to spawn to save code internally is an implementation detail
Sorry, something went wrong.
|
I like adding it to missing APIs but I strongly prefer having examples in APIs to illustrate to users how to use them. (And half the days I don't remember the difference between exec/execFile/fork/spawn myself, and I've been a collaborator for 5 years and a user for ±10 and I implemented the signal support for child_process :D) |
Sorry, something went wrong.
@aduh95 thanks! I added it. |
Sorry, something went wrong.
* Since exec calls execFile and execFile internally calls spawn with options.signal, the signal parameter has been documented under exec as well. * Refactor the description of signal under all the functions. * Add examples of usage of signal under all the functions and add missing requires in the other examples.
benjamingr
left a comment
There was a problem hiding this comment.
LGTM with the lint issues fixed.
Sorry, something went wrong.
The lint errors are fixed. Thanks for reviewing! :) |
Sorry, something went wrong.
|
Can you add the YAML metadata for child_process.fork as well? |
Sorry, something went wrong.
* Since exec calls execFile and execFile internally calls spawn with options.signal, the signal parameter has been documented under exec as well. * Refactor the description of signal under all the functions. * Add examples of usage of signal under all the functions and add missing requires in the other examples. PR-URL: #37528 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* Since exec calls execFile and execFile internally calls spawn with options.signal, the signal parameter has been documented under exec as well. * Refactor the description of signal under all the functions. * Add examples of usage of signal under all the functions and add missing requires in the other examples. PR-URL: #37528 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* Since exec calls execFile and execFile internally calls spawn with options.signal, the signal parameter has been documented under exec as well. * Refactor the description of signal under all the functions. * Add examples of usage of signal under all the functions and add missing requires in the other examples. PR-URL: nodejs#37528 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
* Since exec calls execFile and execFile internally calls spawn with options.signal, the signal parameter has been documented under exec as well. * Refactor the description of signal under all the functions. * Add examples of usage of signal under all the functions and add missing requires in the other examples. PR-URL: #37528 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
options.signal, the signal parameter has been documented under exec
as well.
missing requires in the other examples.