RESOLVED FIXED 38957
scm.py should use self.run instead of run_command
https://bugs.webkit.org/show_bug.cgi?id=38957
Summary scm.py should use self.run instead of run_command
Adam Barth
Reported 2010-05-11 17:24:11 PDT
scm.py should use self.run instead of run_command
Attachments
Patch (19.41 KB, patch)
2010-05-11 17:25 PDT, Adam Barth
no flags
Adam Barth
Comment 1 2010-05-11 17:25:28 PDT
Eric Seidel (no email)
Comment 2 2010-05-11 20:23:26 PDT
Comment on attachment 55784 [details] Patch 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.
Adam Barth
Comment 3 2010-05-14 23:23:19 PDT
> Do you just want **kwargs? Or do you want to manage the default values explicitly? Future patches will make run smarter to handle the args. It's possible we'll want **kwargs at some point though. > 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. :) I'll do that in a future patch. I just want to take it slow. > By use Executive() i mean just create one here locally, even if it's not stored on self. yet. I understand. > r=me assuming your intent was not to change behavior. Please consider the above comments. Thanks for the comments. Sorry I'm ignoring all of them. :P
Adam Barth
Comment 4 2010-05-14 23:29:02 PDT
Comment on attachment 55784 [details] Patch Clearing flags on attachment: 55784 Committed r59522: <http://trac.webkit.org/changeset/59522>
Adam Barth
Comment 5 2010-05-14 23:29:09 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.