[webkit-reviews] review granted: [Bug 135409] [iOS] run-webkit-tests runs webkit-build-directory on every test : [Attachment 236056] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted David Farler <dfarler at apple.com>'s
request for review:
Bug 135409: [iOS] run-webkit-tests runs webkit-build-directory on every test
https://bugs.webkit.org/show_bug.cgi?id=135409

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=236056&action=review


> Tools/ChangeLog:10
> +	   (IOSSimulatorPort.__init__):
> +	   Cache Mac build directory once.

Nit: It seems sufficient to collapse these two line into a single line such
that it reads:

(IOSSimulatorPort.__init__): Cache Mac build directory.

(Notice that I removed the word "once" since this is implied by the use of the
word "cache").

> Tools/ChangeLog:12
> +	   (IOSSimulatorPort.relay_path):
> +	   Use cached build directory.

Nit: Similarly I would write the contents of these two lines in a single line.

> Tools/ChangeLog:14
> +	   (IOSSimulatorPort._path_to_image_diff):
> +	   Use cached build directory.

Ditto.

> Tools/Scripts/webkitpy/port/ios.py:67
> +	   self._mac_config = port_config.Config(self._executive,
self._filesystem, 'mac')

This instance variable is referenced exactly once (on the line below). Unless
you plan to make use of it in subsequent patches in the near future, I suggest
we either inline its value into the line below or make it a local variable.