[webkit-reviews] review granted: [Bug 215702] [webkitcorepy] Add subprocess_utils.run : [Attachment 406938] Patch

[bugzilla-daemon at webkit.org]

dewei_zhu at apple.com has granted Jonathan Bedard <jbedard at apple.com>'s request
for review:
Bug 215702: [webkitcorepy] Add subprocess_utils.run
https://bugs.webkit.org/show_bug.cgi?id=215702

Attachment 406938: Patch

https://bugs.webkit.org/attachment.cgi?id=406938&action=review




--- Comment #3 from dewei_zhu at apple.com ---
Comment on attachment 406938
  --> https://bugs.webkit.org/attachment.cgi?id=406938
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=406938&action=review

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:39
> +	   input = kwargs.pop('input', None)
> +	   capture_output = kwargs.pop('capture_output', False)
> +	   timeout = kwargs.pop('timeout', None)
> +	   check = kwargs.pop('check', False)

Other than timeout, is there any reason we want to extract other variables out
of **kwargs? It looks like the same defaults in the subprocess.run

> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:54
> +		   return subprocess.run(*popenargs, input=input,
capture_output=capture_output, timeout=timeout, check=check, **kwargs)
> +	       return subprocess.run(
> +		   *popenargs,
> +		   input=input,
> +		   capture_output=capture_output,
> +		   timeout=min(timeout or sys.maxsize,
int(math.ceil(difference))),
> +		   check=check,
> +		   **kwargs)

So the only difference is timeout, maybe we can simplify this by doing:
if difference:
    timeout = min(timeout or sys.maxsize, int(math.ceil(difference)))
return subprocess.run(*popenargs, input=input, capture_output=capture_output,
timeout=timeout, check=check, **kwargs)