[webkit-reviews] review denied: [Bug 105391] PerfTest.parse_output does too much : [Attachment 180098] Added the missing change log

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 19 10:04:14 PST 2012


Tony Chang <tony at chromium.org> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 105391: PerfTest.parse_output does too much
https://bugs.webkit.org/show_bug.cgi?id=105391

Attachment 180098: Added the missing change log
https://bugs.webkit.org/attachment.cgi?id=180098&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=180098&action=review


r- for what appears to be a typo.

> Tools/Scripts/webkitpy/performance_tests/perftest.py:175
> +	       if description_match:
> +		   description = description.group('description')

Is this correct?  Shouldn't it be description_match on the right side?	Can we
add a test case with a description?

> Tools/Scripts/webkitpy/performance_tests/perftest.py:209
> +	   return results, description

Nit: Please make a class and return the class.	It makes the calling code much
easier to understand rather than a tuple with unnamed values and makes it
harder to accidentally return the wrong type.


More information about the webkit-reviews mailing list