Bug 73637 - [Refactoring] In webkitpy/bindings/main.py, replace subprocess.Popen() with Executive
Summary: [Refactoring] In webkitpy/bindings/main.py, replace subprocess.Popen() with E...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-02 02:19 PST by Kentaro Hara
Modified: 2011-12-06 00:05 PST (History)
4 users (show)

See Also:

Attachments
Patch (6.15 KB, patch)
2011-12-02 02:29 PST, Kentaro Hara 		no flags Details | Formatted Diff | Diff
Patch (5.23 KB, patch)
2011-12-05 18:09 PST, Kentaro Hara 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-12-02 02:19:13 PST 
We can replace subprocess.Popen() with Executive, similar to other Python scripts in webkitpy/.
Comment 1 Kentaro Hara 2011-12-02 02:29:29 PST 
Created attachment 117593 [details]
Patch
Comment 2 Adam Barth 2011-12-02 10:36:59 PST 
I forget whether run_command or run_and_throw_if_failed is preferred.

@Eric?
Comment 3 Eric Seidel (no email) 2011-12-02 10:42:49 PST 
(In reply to comment #2)
> I forget whether run_command or run_and_throw_if_failed is preferred.
> 
> @Eric?

run_command.
Comment 4 Adam Barth 2011-12-05 16:06:50 PST 
Comment on attachment 117593 [details]
Patch

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

> Tools/Scripts/webkitpy/common/system/executive.py:403
> +                    return_exit_code_and_output=False,

Why not just catch the exception?  That's what we usually do in these cases.  The ScriptError has all the data.
Comment 5 Kentaro Hara 2011-12-05 18:09:59 PST 
Created attachment 117972 [details]
Patch
Comment 6 Adam Barth 2011-12-05 18:49:27 PST 
Comment on attachment 117972 [details]
Patch

Thanks.
Comment 7 WebKit Review Bot 2011-12-06 00:05:29 PST 
Comment on attachment 117972 [details]
Patch

Clearing flags on attachment: 117972

Committed r102105: <http://trac.webkit.org/changeset/102105>
Comment 8 WebKit Review Bot 2011-12-06 00:05:35 PST 
All reviewed patches have been landed.  Closing bug.