Bug 37408 - Break new-run-webkit-tests options into groups for easier re-use and possible relocation
Summary: Break new-run-webkit-tests options into groups for easier re-use and possible...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-11 01:15 PDT by Eric Seidel (no email)
Modified: 2010-04-12 16:39 PDT (History)
4 users (show)

See Also:


Attachments
Patch (17.92 KB, patch)
2010-04-11 01:22 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff
Patch for landing (20.12 KB, patch)
2010-04-11 19:04 PDT, Eric Seidel (no email)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-04-11 01:15:56 PDT
Break new-run-webkit-tests options into groups for easier re-use and possible relocation
Comment 1 Eric Seidel (no email) 2010-04-11 01:22:47 PDT
Created attachment 53081 [details]
Patch
Comment 2 Eric Seidel (no email) 2010-04-11 01:24:13 PDT
One of the mild inconsistencies in this code currently is punctuation and capitalization of option help strings.
Comment 3 Mark Rowe (bdash) 2010-04-11 02:14:26 PDT
Any chance that the misspelling of “deprecated" can be fixed while you’re touching it?
Comment 4 Adam Barth 2010-04-11 08:51:09 PDT
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?
Comment 5 Dirk Pranke 2010-04-11 11:41:13 PDT
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.
Comment 6 Eric Seidel (no email) 2010-04-11 11:50:10 PDT
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.
Comment 7 Eric Seidel (no email) 2010-04-11 11:50:38 PDT
I've gone through and commented in each group about the options missing from new-run-webkit-tests vs. old-run-webkit-tests.
Comment 8 Eric Seidel (no email) 2010-04-11 19:04:06 PDT
Created attachment 53125 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2010-04-11 19:16:51 PDT
Comment on attachment 53125 [details]
Patch for landing

Clearing flags on attachment: 53125

Committed r57463: <http://trac.webkit.org/changeset/57463>
Comment 10 WebKit Commit Bot 2010-04-11 19:16:57 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Dirk Pranke 2010-04-12 16:39:28 PDT
Um, this wasn't anywhere near formatted for 80-columns. Assuming this was a mistake, please be more careful next time.