LGTM. I'm pretty much rubber-stamping this -- it's just a test driver; as long as it does what you want it to do, no need to obsess over implementation details
:-)

https://codereview.chromium.org/293023006/diff/230004/tools/run_benchmarks.py
File tools/run_benchmarks.py (right):

https://codereview.chromium.org/293023006/diff/230004/tools/run_benchmarks.py#newcode244
tools/run_benchmarks.py:244: # TODO(machenbach): This requires +.exe if
ran on windows.
nit: s/ran/run/

https://codereview.chromium.org/293023006/diff/230004/tools/run_benchmarks.py#newcode346
tools/run_benchmarks.py:346: if not options.mode.lower() in ["debug",
"release", "optdebug"]:
arguably running benchmarks in anything other than release mode doesn't
make sense anyway

https://codereview.chromium.org/293023006/diff/230004/tools/run_benchmarks.py#newcode387
tools/run_benchmarks.py:387: output =
commands.Execute(runnable.GetCommand(shell_dir), timeout=60)
for slow (mobile) devices 60 seconds might be too tight

https://codereview.chromium.org/293023006/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to