[webkit-reviews] review denied: [Bug 78572] webkitpy: add a worker_args concept to start_worker() : [Attachment 126902] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 78572: webkitpy: add a worker_args concept to start_worker()
https://bugs.webkit.org/show_bug.cgi?id=78572

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

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


> Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:108
> +	   If the worker is being run in the same thread/process as the
manager, it may be
> +	   passed an optional inline_arg from the manager. Use of this object
is discouraged
> +	   as workers should be able to run in a shared-nothing environment in
separate processes."""

Rather than passing a separate param, why not make |port| an optional param for
WorkerParameters.  Or add a setter method to WorkerParameters that adds port? 
It's weird to have this param hang around for some callers.

> Tools/Scripts/webkitpy/layout_tests/controllers/worker.py:47
> +class WorkerArgs(object):

WorkerArguments or WorkerParameters

> Tools/Scripts/webkitpy/layout_tests/controllers/worker.py:55
> +    def __init__(self, worker_connection, worker_arg_obj):

worker_arg_obj -> worker_arguments or worker_parameters.  _obj doesn't add
anything.

> Tools/Scripts/webkitpy/layout_tests/controllers/worker.py:56
> +	   super(Worker, self).__init__(worker_connection, worker_arg_obj)

Nit: Why not be explicit about which init we're calling?