[Webkit-unassigned] [Bug 37836] Attempt to make new-run-webkit-tests --help more sane

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 20 15:06:59 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=37836





--- Comment #13 from Dirk Pranke <dpranke at chromium.org>  2010-04-20 15:06:58 PST ---
(From update of attachment 53757)

> diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> index 68fd865..93bab69 100755
> --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
> +class HiddenOptionGroup(optparse.OptionGroup):
>  def _compat_shim_callback(option, opt_str, value, parser):
> +def _deprecated_shim_callback(option, opt_str, value, parser, new_option):
> +def _deprecated_option(parser, option_name, long_name=None, new_option_name=None):
> +def _split_option_name(option_name):
> +def _compare_option_names(a, b):
> +def _compare_options(a, b):
> +def _add_option_group(parser, title, options):
> +def _add_hidden_options(parser, options):

Please add comments and/or docstrings to all of this code. Ideally this would
all be moved into a separate module (but you're already onto that). The intent
and implementation of this is entirely non-obvious.

>      logging_options = [
> +        # FIXME: This option is super-confusing and should be hidden or removed.


My plan is to add a separate --help-logging message to get the details of this.
Variable logging is important and shouldn't be hidden completely.

>          optparse.make_option("--sources", action="store_true",
> -            help="show expected result file path for each test " +
> -                 "(implies --verbose)"),

I think we can eliminate --sources

>      # FIXME: These options should move onto the ChromiumPort.

I don't understand how you expect to move options into different source files.
Do expect this main routine to gather the arguments from each file somehow?

Also, in general, editorial cleanup is often better separated into a separate
patch from functional cleanup and refactoring. That way it's easier to tell
what's important when reviewing the patch.

Note that --nocheck-sys-deps isn't Chromium-specific. The call is made on every
port.

Otherwise, looks reasonable.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list