[webkit-reviews] review granted: [Bug 178059] Improve --help documentation and add --list-plans to show available benchmarks. : [Attachment 323122] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Oct 7 22:25:57 PDT 2017
Ryosuke Niwa <rniwa at webkit.org> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 178059: Improve --help documentation and add --list-plans to show available
benchmarks.
https://bugs.webkit.org/show_bug.cgi?id=178059
Attachment 323122: Patch
https://bugs.webkit.org/attachment.cgi?id=323122&action=review
--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 323122
--> https://bugs.webkit.org/attachment.cgi?id=323122
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=323122&action=review
> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:64
> + @staticmethod
> + def available_plans():
I don't think this function belongs to benchmark_runner class. This class
shouldn't know anything about where plan files are located.
We should keep it in run_benchmark.py.
> Tools/Scripts/webkitpy/benchmark_runner/benchmark_runner.py:66
> + plans = [plan_file.replace(".plan", "") for plan_file in
os.listdir(plan_directory) if plan_file.endswith(".plan")]
We should continue to use os.path.splitext(plan_file)[0]
This code replaces any string of ".plan" in its name like
"x*.plan*ebench.plan".
> Tools/Scripts/webkitpy/benchmark_runner/run_benchmark.py:88
> - raise Exception('Cant find any .plan file in directory %s' %
plandir)
> + raise Exception('Cant find any .plan file in directory %s' %
BenchmarkRunner.available_plans())
I don't think this change makes sense. This error messages is about not being
able to find any plans.
We should show the directory name instead.
More information about the webkit-reviews
mailing list