chore: use tinyexec - #1213
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/sv/c/1ab1830738c64cd5455a7ec612d1b1f5c64e339bOpen in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
|
sacrosanctic
left a comment
sacrosanctic
left a comment
There was a problem hiding this comment.
Not sure if these are supposed to throw.
| await exec(command, args, { nodeOptions: { cwd }, throwOnError: true }); | ||
| return {}; | ||
| } catch (e) { | ||
| // @ts-expect-error tinyexec rethrows the spawn error as-is |
There was a problem hiding this comment.
// @ts-expect-error tinyexec rethrows the spawn error as-is
I know you didn't change this catch-block, but is this still correct? Looking through NonZeroExitError, I don't see e.code as an available prop.
There was a problem hiding this comment.
e.code is a node error. I'll update the code to clarify this.
| } | ||
|
|
||
| if (dockerInstalled) execSync('docker compose up --detach', { cwd, stdio: 'pipe' }); | ||
| if (dockerInstalled) execSync('docker', ['compose', 'up', '--detach'], { nodeOptions: { cwd } }); |
There was a problem hiding this comment.
this one should probably throw (or at least be handled in some way) as well if we're depending on it running successfully
|
|
||
| fs.writeFileSync(path.join(test_workspace_dir, 'pnpm-workspace.yaml'), 'packages:\n - ./*\n'); | ||
|
|
||
| const exec_async = promisify(nodeExec); |
There was a problem hiding this comment.
hmmm, there was a reason we used this instead due to failing tests, but if it's no longer failing then we're probably good
Co-authored-by: CokaKoala <31664583+AdrianGonz97@users.noreply.github.com>
|
I just learned that execsync still throws even when it is false if the command itself is invalid... |
2 participants
Closes #
Description
Checklist