[webkit-reviews] review granted: [Bug 171881] webkitpy: Run sample/spindump on iOS devices : [Attachment 312317] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 171881: webkitpy: Run sample/spindump on iOS devices
https://bugs.webkit.org/show_bug.cgi?id=171881

Attachment 312317: Patch

https://bugs.webkit.org/attachment.cgi?id=312317&action=review




--- Comment #14 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 312317
  --> https://bugs.webkit.org/attachment.cgi?id=312317
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=312317&action=review

> Tools/Scripts/webkitpy/port/darwin.py:165
> +	   command = ['/usr/bin/sudo', '-n'] + command if
self.host.platform.is_mac() else command

I take it you disagreed with by suggestion in comment #10 that it would be more
idiomatic to write this line as a single if statement without an else clause:

if self.host.platform.is_mac():
    command = ['/usr/bin/sudo', '-n'] + command

?

> Tools/Scripts/webkitpy/port/ios_device_unittest.py:50
> +

I do not see the value of having this empty line and its presence is
inconsistent with the lack of an empty line at the beginning of
test_sample_process_exception(). I suggest we remove this empty line.

> Tools/Scripts/webkitpy/port/ios_device_unittest.py:63
> +

Ditto.