[webkit-reviews] review granted: [Bug 38957] scm.py should use self.run instead of run_command : [Attachment 55784] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 38957: scm.py should use self.run instead of run_command
https://bugs.webkit.org/show_bug.cgi?id=38957

Attachment 55784: Patch
https://bugs.webkit.org/attachment.cgi?id=55784&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
WebKitTools/Scripts/webkitpy/common/checkout/scm.py:103
 +	def run(self, args, cwd=None, input=None, error_handler=None,
return_exit_code=False, return_stderr=True, decode_output=True):
Do you just want **kwargs?  Or do you want to manage the default values
explicitly?

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:105
 +	    # FIXME: We should use Executive.
Why not just use Executive() now anyway?  No need to add more clients to the
raw version, then we have less to import to this file anyway. :)

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:105
 +	    # FIXME: We should use Executive.
By use Executive() i mean just create one here locally, even if it's not stored
on self. yet.

WebKitTools/Scripts/webkitpy/common/checkout/scm.py:130
 +		print self.run(self.status_command(),
error_handler=Executive.ignore_error)
Since you're not using **kwargs, and instead specifying all the defaults, it's
more difficult for me to tell if you've changed behavior here.

r=me assuming your intent was not to change behavior.  Please consider the
above comments.