[webkit-reviews] review denied: [Bug 62256] webkitpy: clean up code prior to functional changes for server startup/shutdown : [Attachment 96840] update w/ more review feedback

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 62256: webkitpy: clean up code prior to functional changes for server
startup/shutdown
https://bugs.webkit.org/show_bug.cgi?id=62256

Attachment 96840: update w/ more review feedback
https://bugs.webkit.org/attachment.cgi?id=96840&action=review

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

> Tools/ChangeLog:12
> +	   removing unused code paths and functions, using 'name' and
> +	   'pid_file' as public properties on the server objects, and using

Nit: Please update the part about 'name' and 'pid_file' being public
properties.

> Tools/Scripts/webkitpy/layout_tests/port/apache_http_server.py:44
>  class LayoutTestApacheHttpd(http_server_base.HttpServerBase):
> +    name = 'apache'

This is no longer needed, right?

> Tools/Scripts/webkitpy/layout_tests/port/http_server.py:91
> +	       raise http_server_base.ServerError('Lighttpd already running')

Nit: Should this use self._name?

> Tools/Scripts/webkitpy/layout_tests/port/websocket_server.py:200
> +	   if self.pid_file:
> +	       self._filesystem.write_text_file(self.pid_file, "%d" %
self._process.pid)

This is now self._pid_file, right?

> Tools/Scripts/webkitpy/layout_tests/port/websocket_server.py:210
> +	   elif self.pid_file:
> +	       pid = int(self._filesystem.read_text_file(self.pid_file))

Ditto.