-
-
Notifications
You must be signed in to change notification settings - Fork 367
Closed
Labels
Description
So #791 added the core support for subprocesses, but at the last minute we had doubts about copying subprocess.run
and decided to split out the high level convenience API into a separate PR: #791 (comment)
Some issues to consider (it's worth reading the whole discussion linked above):
- probably make
check=True
as the default. (Might be worth considering making the name more descriptive, so people would writeallow_failure=True
?) - Passing
stdout=PIPE
to mean "capture the output" is pretty confusing (basically just dumping an implementation detail into the public API). Maybecapture_stdout=
/capture_stderr=
as boolean kwargs? Should we havecapture=
as a shorthand for setting both at once? - If we're changing the default for
check
, we should change the name, because that's just enough of a change for people to trip over.trio.run_process
is a good name. - Should we do anything about the weird mess around
shell=True
and string-versus-list commands?