Bug 82158

Summary: add a Tree abstraction to test-webkitpy to better encapsulate things
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
fix typos
none
rename _Tree to TestDirectoryTree none

Dirk Pranke
Reported 2012-03-25 19:01:03 PDT
add a Tree abstraction to test-webkitpy to better encapsulate things
Attachments
Patch (9.81 KB, patch)
2012-03-25 19:08 PDT, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
fix typos (9.85 KB, patch)
2012-03-25 19:10 PDT, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
rename _Tree to TestDirectoryTree (9.98 KB, patch)
2012-03-26 14:56 PDT, Dirk Pranke 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dirk Pranke
Comment 1 2012-03-25 19:08:28 PDT
Created attachment 133712 [details] Patch
Dirk Pranke
Comment 2 2012-03-25 19:10:38 PDT
Created attachment 133713 [details] fix typos
Adam Barth
Comment 3 2012-03-26 10:26:23 PDT
Comment on attachment 133713 [details] fix typos View in context: https://bugs.webkit.org/attachment.cgi?id=133713&action=review > Tools/Scripts/webkitpy/test/main.py:238 > +class _Tree(object): Do you mean a DirectoryTree? "Tree" is a very broad name. This code seems very specific to Python packages.
Dirk Pranke
Comment 4 2012-03-26 11:11:44 PDT
Comment on attachment 133713 [details] fix typos View in context: https://bugs.webkit.org/attachment.cgi?id=133713&action=review >> Tools/Scripts/webkitpy/test/main.py:238 >> +class _Tree(object): > > Do you mean a DirectoryTree? "Tree" is a very broad name. This code seems very specific to Python packages. Yes, it refers to directory trees and is intended to by very python-specific. DirectoryTree is a fine name; other suggestions welcome.
Adam Barth
Comment 5 2012-03-26 14:15:31 PDT
Comment on attachment 133713 [details] fix typos I'd rename _Tree to something more specific (and maybe put it at the top of the file?). There's an argument that says we should put the new class in its own module, which would have the benefit of making test-webkitpy smaller. If we did that we, we'd probably want to use filesystem rather than os.path. I think it's a question of how far you'd like to shave this yak.
Dirk Pranke
Comment 6 2012-03-26 14:22:28 PDT
(In reply to comment #5) > (From update of attachment 133713 [details]) > I'd rename _Tree to something more specific (and maybe put it at the top of the file?). There's an argument that says we should put the new class in its own module, which would have the benefit of making test-webkitpy smaller. If we did that we, we'd probably want to use filesystem rather than os.path. I think it's a question of how far you'd like to shave this yak. Okay, I will rename and move it. I don't see much advantage to putting it in a different file, since I don't think anyone else will want to use it, and main.py isn't very big as it is. Thanks for the review!
Dirk Pranke
Comment 7 2012-03-26 14:56:06 PDT
Created attachment 133899 [details] rename _Tree to TestDirectoryTree
Dirk Pranke
Comment 8 2012-03-26 14:58:25 PDT
Committed r112150: <http://trac.webkit.org/changeset/112150>
Note You need to log in before you can comment on or make changes to this bug.