[webkit-reviews] review granted: [Bug 201954] Python 3: Add test-webkitpy for Python 3 : [Attachment 380020] Patch

[bugzilla-daemon at webkit.org]

Aakash Jain <aakash_jain at apple.com> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 201954: Python 3: Add test-webkitpy for Python 3
https://bugs.webkit.org/show_bug.cgi?id=201954

Attachment 380020: Patch

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




--- Comment #6 from Aakash Jain <aakash_jain at apple.com> ---
Comment on attachment 380020
  --> https://bugs.webkit.org/attachment.cgi?id=380020
Patch

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

r+ with comments.

> Tools/Scripts/test-webkitpy-python3:11
> +PYTHON3_DIRECTORIES = [

might consider renaming it to something like this to be more readable/clear:
PYTHON3_COMPATIBLE_DIRECTORIES

> Tools/Scripts/test-webkitpy-python3:17
> +def list_to_string(lst):

lst is confusing (it might be read as first). alternatives: input, input_list.

Also this method isn't really required, it's used only once, you can simply use
following at that place (although this one generates string which is slightly
more readable, since it adds 'and').

', '.join(list)

> Tools/Scripts/test-webkitpy-python3:50
> +	   for tst in module_suite if module_suite else []:

Nit: tst => test

Following might be more cleaner:

for test in (module_suite or []):

> Tools/Scripts/test-webkitpy-python3:54
> +	   raise RuntimeError('No tests matching...')

Nit: 'No tests matching...' => 'No matching tests found.'