[webkit-reviews] review denied: [Bug 76278] [NRWT] Support --ignore-metrics : [Attachment 122434] fixed style

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Balazs Kelemen <kbalazs at webkit.org>'s
request for review:
Bug 76278: [NRWT] Support --ignore-metrics
https://bugs.webkit.org/show_bug.cgi?id=76278

Attachment 122434: fixed style
https://bugs.webkit.org/attachment.cgi?id=122434&action=review

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


Some minor python style nits.  Overall, the patch seems OK.

> Tools/Scripts/webkitpy/layout_tests/port/driver.py:67
> +	   strip_patterns = self.__class__.strip_patterns
> +	   if strip_patterns == []:

You can just define the patterns above and avoid the if check.	Also,
self.strip_patterns will automatically fall back to
self.__class__.strip_patterns.

> Tools/Scripts/webkitpy/layout_tests/port/driver.py:80
> +	       strip_patterns.append((re.compile('\n( *)"\s+'), r'\n\1"'))

r'\n\1' becomes \n\1 (no newline, actually backslash-n-backslash-1).  I think
you meant to do '\n\\1' instead.  I'm not sure if a test case covers this
regex.

> Tools/Scripts/webkitpy/layout_tests/port/driver_unittest.py:57
> +	       ('text run at (0,0) width 109: ".one {color: green;}"\n\
> +	     text run at (109,0) width 0: " "\n\
> +	     text run at (0,17) width 81: ".1 {color: red;}"\n\
> +	     text run at (81,17) width 0: " "\n\

You can use triple quotes and avoid the need to manually add \n\.