Bug 37408 – Break new-run-webkit-tests options into groups for easier re-use and possible relocation

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Eric Seidel (no email)]

Break new-run-webkit-tests options into groups for easier re-use and possible relocation

[Eric Seidel (no email)]

Created attachment 53081 [details]
Patch

[Eric Seidel (no email)]

One of the mild inconsistencies in this code currently is punctuation and capitalization of option help strings.

[Mark Rowe (bdash)]

Any chance that the misspelling of “deprecated" can be fixed while you’re touching it?

[Adam Barth]

Comment on attachment 53081 [details]
Patch

Ok.  Please fix the spelling error that Mark pointed out.  Also configuration_options seems to occur twice.  Can we start by sharing that?

[Dirk Pranke]

Comment on attachment 53081 [details]
Patch

Personally, I prefer options lists to be alphabetized, especially for the short flags, but I can see what you're trying to do. Can you maybe at least alphabetize in each group?

Down the road it might be good to have the groups share common prefixes in the long flags, to make this clearer.

Also, if you are really brave, it was part of my original intent to pull some options from the port object itself so, for example, Chromium-specific flags would live in chromium.py, and not clutter up the main code. This would require you to parse the flags in two passes, the first to figure out which port to run, and then second to re-parse the flags with the generic list plus a list pulled from the port. This would also address port-specific default values more cleanly.

[Eric Seidel (no email)]

I agree we should move the options to chromium.py.  I don't think we should parse in two passes, as they would be hidden from the main help.  I think we just want to list the Chromium-only and Mac-only and Qt-only, etc. options separately in the main help.

[Eric Seidel (no email)]

I've gone through and commented in each group about the options missing from new-run-webkit-tests vs. old-run-webkit-tests.

[Eric Seidel (no email)]

Created attachment 53125 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 53125 [details]
Patch for landing

Clearing flags on attachment: 53125

Committed r57463: <http://trac.webkit.org/changeset/57463>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dirk Pranke]

Um, this wasn't anywhere near formatted for 80-columns. Assuming this was a mistake, please be more careful next time.