Bug 101140 – webkitpy: clean up options for specifying multiple platforms at once

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

[Dirk Pranke]

webkitpy: clean up options for specifying multiple platforms at once

[Dirk Pranke]

Created attachment 172194 [details]
Patch

[Ojan Vafai]

Comment on attachment 172194 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/port/factory.py:39
> +def platform_options(globs=False):

This variable name doesn't read like a boolean. It confused me at first reading the code. How about "allow_globs" or "use_globs"?

> Tools/Scripts/webkitpy/layout_tests/port/factory.py:44
> +            const=('chromium' if globs else 'chromium'),

I think you're missing a *.

> Tools/Scripts/webkitpy/layout_tests/port/factory.py:138
> +        If platform is specified, we will glob-match all ports"""

Missing a "not"?

> Tools/Scripts/webkitpy/tool/commands/rebaseline.py:-356
> -        return super(RebaselineJson, self).__init__(options=[

lol whoops

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 172194 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172194&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/port/factory.py:39
> > +def platform_options(globs=False):
> 
> This variable name doesn't read like a boolean. It confused me at first reading the code. How about "allow_globs" or "use_globs"?
> 

Makes sense. 

> > Tools/Scripts/webkitpy/layout_tests/port/factory.py:44
> > +            const=('chromium' if globs else 'chromium'),
> 
> I think you're missing a *.
>

Good catch.
 
> > Tools/Scripts/webkitpy/layout_tests/port/factory.py:138
> > +        If platform is specified, we will glob-match all ports"""
> 
> Missing a "not"?
> 

Right.

[Dirk Pranke]

Committed r133507: <http://trac.webkit.org/changeset/133507>