Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

promisified exec and execFile should return a promise with ChildProcess instance attached #880

Merged
merged 4 commits into from
Mar 26, 2021
Merged

promisified exec and execFile should return a promise with ChildProcess instance attached #880

merged 4 commits into from
Mar 26, 2021

Conversation

onip
Copy link
Contributor

@onip onip commented Mar 30, 2020

as per documentation require('child_process').exec and require('child_process').execFile should return a promise with attached a the require('child_process').ChildProcess instance attached to the .child property.

this PR is an adaption of the same code used inside nodejs

> require('child_process').exec[require('util').promisify.custom].toString ()
'(...args) => {\n' +
  '    let resolve;\n' +
  '    let reject;\n' +
  '    const promise = new Promise((res, rej) => {\n' +
  '      resolve = res;\n' +
  '      reject = rej;\n' +
  '    });\n' +
  '\n' +
  '    promise.child = orig(...args, (err, stdout, stderr) => {\n' +
  '      if (err !== null) {\n' +
  '        err.stdout = stdout;\n' +
  '        err.stderr = stderr;\n' +
  '        reject(err);\n' +
  '      } else {\n' +
  '        resolve({ stdout, stderr });\n' +
  '      }\n' +
  '    });\n' +
  '\n' +
  '    return promise;\n' +
  '  }'

@onip
Copy link
Contributor Author

onip commented Mar 30, 2020

resolves #881

@github-actions
Copy link

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

@github-actions github-actions bot added the Stale label Mar 25, 2021
@robertsLando
Copy link
Contributor

@onip Could you fix conflicts please? This is ok to merge once conflicts are fixed and tests pass @leerob

@robertsLando robertsLando added ok to merge PR changes are ok and removed Stale labels Mar 26, 2021
@onip
Copy link
Contributor Author

onip commented Mar 26, 2021

master merged in

prelude/bootstrap.js Outdated Show resolved Hide resolved
Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

LGTM. @leerob merge this when possible please

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ok to merge PR changes are ok
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants