[webkit-reviews] review granted: [Bug 54431] nrwt multiprocessing: fix various windows-related regressions : [Attachment 82412] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 15 10:30:11 PST 2011


Tony Chang <tony at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 54431: nrwt multiprocessing: fix various windows-related regressions
https://bugs.webkit.org/show_bug.cgi?id=54431

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

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

Seems fine to try.

> Tools/Scripts/new-run-webkit-tests:40
> +    # (which means a file in sys.path that ends in .py). We use the main
> +    # file in the layout_tests subdirectory, and we also make sure that
> +    # sys.path / PYTHONPATH is set and propagating correctly.

Nit: The second sentence seems to be repeating what is already stated in the
first sentence.

> Tools/Scripts/new-run-webkit-tests:43
> +    script_dir = os.path.dirname(os.path.abspath(__file__))
> +    env = os.environ
> +    env['PYTHONPATH'] = script_dir

Should we append the old PYTHONPATH to script_dir?

>
Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unitte
st.py:47
> +# In order to reliably control when child workers are starting and stopping,

> +# we a pair of global variables to hold queues used for messaging. Ideally

Nit: we a pair -> we use a pair?

>
Tools/Scripts/webkitpy/layout_tests/layout_package/manager_worker_broker_unitte
st.py:51
>  

Nit: Remove blank line between comment and variables.


More information about the webkit-reviews mailing list