Bug 74896

Summary: webkitpy: remove port.executive, port.filesystem, port.user properties
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
remove TestConfiguration.from_port() eric: review+

Dirk Pranke
Reported 2011-12-19 16:04:49 PST
webkitpy: remove port.executive, port.filesystem, port.user properties
Attachments
Patch (17.48 KB, patch)
2011-12-19 16:07 PST, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
remove TestConfiguration.from_port() (20.46 KB, patch)
2011-12-19 16:53 PST, Dirk Pranke 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2011-12-19 16:07:33 PST
Created attachment 119936 [details] Patch
Eric Seidel (no email)
Comment 2 2011-12-19 16:37:17 PST
Comment on attachment 119936 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119936&action=review OK. I'm surprised the diff is this small. > Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:43 > - build_type = port.options.configuration.lower() > + build_type = port.get_option('configuration').lower() I think the previous version was cleaner. Certainly for testing code where we're not trying to have dynamic support for different option configurations.
Dirk Pranke
Comment 3 2011-12-19 16:53:00 PST
Created attachment 119953 [details] remove TestConfiguration.from_port()
Dirk Pranke
Comment 4 2011-12-19 16:55:40 PST
(In reply to comment #2) > (From update of attachment 119936 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=119936&action=review > > OK. I'm surprised the diff is this small. > Yeah, there weren't too many references to it. > > Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:43 > > - build_type = port.options.configuration.lower() > > + build_type = port.get_option('configuration').lower() > > I think the previous version was cleaner. Certainly for testing code where we're not trying to have dynamic support for different option configurations. This comment caused me to actually look at what was going on in this routine. It turns out Dimitri had refactored an earlier version of the TestConfiguration constructor and didn't quite get the refactoring as clean as it could be. By finishing that (and updating Port.test_configuration() to just call the 4-arg constructor directly) I can delete this classmethod and better encapsulate things, so it's even cleaner now.
Eric Seidel (no email)
Comment 5 2011-12-19 17:12:28 PST
Comment on attachment 119953 [details] remove TestConfiguration.from_port() OK.
Dirk Pranke
Comment 6 2011-12-19 17:16:39 PST
Committed r103289: <http://trac.webkit.org/changeset/103289>
Note You need to log in before you can comment on or make changes to this bug.