RESOLVED FIXED 146125
Extend run-benchmark script capability to supoort mobile device
https://bugs.webkit.org/show_bug.cgi?id=146125
Summary Extend run-benchmark script capability to supoort mobile device
dewei_zhu
Reported 2015-06-18 15:37:37 PDT
Extend run-benchmark script capability to supoort mobile device
Attachments
Patch (12.18 KB, patch)
2015-06-18 15:39 PDT, dewei_zhu
no flags
Patch (12.56 KB, patch)
2015-06-18 18:23 PDT, dewei_zhu
no flags
Patch (12.54 KB, patch)
2015-06-18 18:40 PDT, dewei_zhu
no flags
dewei_zhu
Comment 1 2015-06-18 15:39:19 PDT
Ryosuke Niwa
Comment 2 2015-06-18 17:46:29 PDT
Comment on attachment 255143 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255143&action=review > Tools/ChangeLog:7 > + You should explain what kind of code changes you're making here. See other change log entires. > Tools/Scripts/run-benchmark:14 > _log.addHandler(ch) This logging code should also be moved into some helper function in run_benchmark.py > Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:29 > + _log.info('Initializing benchmark runner') This logging is probably not useful. Remove it? > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:9 > - def prepareEnv(self): > + def prepareEnv(self, udid=None): Why don't we stick with the same "deviceID"? > Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py:13 > +# _log.setLevel(logging.INFO) What's going on with these commented out code? r- because of this.
dewei_zhu
Comment 3 2015-06-18 18:23:51 PDT
Ryosuke Niwa
Comment 4 2015-06-18 18:31:14 PDT
Comment on attachment 255159 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255159&action=review > Tools/ChangeLog:8 > + Extend capability of run-benchmark to support mobile device You need a period after the sentence. > Tools/ChangeLog:11 > + Correct typo. > + * Scripts/run-benchmark: You need a blank line between the line line of the description and the list of files. > Tools/Scripts/webkitpy/benchmark_runner/browser_driver/browser_driver.py:9 > - def prepareEnv(self): > + def prepareEnv(self, deviceID=None): We don't need default arguments in these functions because we always call it with deviceID.
dewei_zhu
Comment 5 2015-06-18 18:40:08 PDT
WebKit Commit Bot
Comment 6 2015-06-18 19:33:22 PDT
Comment on attachment 255162 [details] Patch Clearing flags on attachment: 255162 Committed r185732: <http://trac.webkit.org/changeset/185732>
WebKit Commit Bot
Comment 7 2015-06-18 19:33:27 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.