[webkit-reviews] review granted: [Bug 103581] run-perf-tests --chromium-android should not require adb in my path : [Attachment 176725] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Peter Beverloo <peter at chromium.org>'s
request for review:
Bug 103581: run-perf-tests --chromium-android should not require adb in my path
https://bugs.webkit.org/show_bug.cgi?id=103581

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=176725&action=review


Lgtm

> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:272
> +	   assert provided_version, 'Please be sure to have a Chromium-Android
check-out before running layout tests.'

I might say something like: path/to/adb is missing.  Are you sure you ran
update-webkit --chromium-android?

> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:274
> +	   if not path_version or provided_version > path_version:

You might log in this rare case.

> Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py:331
> +	   try:

This try is redundant. Also I think you can pass ignore_error=true instead of
having to dig up a handler function