Move webkitpy/mock.py into webkitpy/thirdparty. I discovered this was third-party while working on this-- https://bugs.webkit.org/show_bug.cgi?id=35465 (check-webkit-style should check license text)
Created attachment 49701 [details] Proposed patch
Attachment 49701 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 Skipping input 'WebKitTools/Scripts/webkitpy/mock.py': Can't open for reading WebKitTools/Scripts/webkitpy/thirdparty/mock.py:59: whitespace before ')' [pep8/E202] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:64: expected 2 blank lines, found 1 [pep8/E302] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:84: expected 2 blank lines, found 1 [pep8/E302] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:88: expected 2 blank lines, found 1 [pep8/E302] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:91: expected 2 blank lines, found 1 [pep8/E302] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:115: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:127: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:138: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:171: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:174: line too long (80 characters) [pep8/E501] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:187: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:188: line too long (115 characters) [pep8/E501] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:220: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:246: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:259: line too long (86 characters) [pep8/E501] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:263: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:282: too many blank lines (2) [pep8/E303] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:298: line too long (85 characters) [pep8/E501] [5] WebKitTools/Scripts/webkitpy/thirdparty/mock.py:304: too many blank lines (3) [pep8/E303] [5] Total errors found: 19 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #2) > Attachment 49701 [details] did not pass style-queue: > > Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 > Skipping input 'WebKitTools/Scripts/webkitpy/mock.py': Can't open for reading > WebKitTools/Scripts/webkitpy/thirdparty/mock.py:59: whitespace before ')' > [pep8/E202] [5] I filed a report for this here (to exempt webkitpy/thirdparty from most pep8 checks): https://bugs.webkit.org/show_bug.cgi?id=35497
Comment on attachment 49701 [details] Proposed patch Why not just fix all the commands? webkitpy/commands_references.py Seems cleaner to just fix all the commands instead of using commands_references. Otherwise this looks fine. I think ..foo_references is actually quite hard to read. And it's inconsistent throughout all our files. I think we should just import these directly, no?
(In reply to comment #4) Thanks a lot for the review and comments, Eric! > (From update of attachment 49701 [details]) > Why not just fix all the commands? > webkitpy/commands_references.py I've explained some of the reasons for this pattern to Adam in the past: https://bugs.webkit.org/show_bug.cgi?id=33791#c3 I think this pattern is preferable because it helps prevent the number of intra-package references from getting out of control -- especially as webkitpy grows. This makes it easier to move folders around in the repository since there are fewer files to touch: Modules in a package don't have to know the location of every intra-package reference. They just have to know the location of the package's associated reference file. Similarly, if you want to move the location of a referenced package, you don't have to touch large numbers of files. You just have to update the references file. It also helps to understand what packages and functionality a nested package depends on, because the reference file collects them in one place. > I think ..foo_references is actually quite hard to read. And it's inconsistent > throughout all our files. I think we should just import these directly, no? Yeah, I agree -- thanks. (PEP8 does, too, I found out later.) I actually went through the process of changing all those relative references when I was getting test-webkitpy working with Python 2.4 (since 2.4 doesn't support relative references). Fortunately we don't have too many right now.
Created attachment 50148 [details] Proposed patch 2 I replaced the relative references with absolute references (e.g. ..command_references with webkitpy.command_references) and left the references files alone after explaining their purpose. We can continue discussing that though if you want. Thanks for considering.
Comment on attachment 50148 [details] Proposed patch 2 Nice. Eventually we need to move more stuff out of the webkitpy root directory.
(In reply to comment #7) > (From update of attachment 50148 [details]) > Nice. Eventually we need to move more stuff out of the webkitpy root > directory. Thanks, Adam! I'll create a master bug for that mini-project so we can discuss how we'd like to organize things and because the move may be spread over more than one patch.
In one of your other patches, you made a "patch" directory. It might make sense to move webkit-patch specific files there.
Manually committed (via svn commit): http://trac.webkit.org/changeset/55968
(In reply to comment #9) > In one of your other patches, you made a "patch" directory. It might make > sense to move webkit-patch specific files there. Sounds good. I might need your help in determining which files and folders are webkit-patch specific.
(In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 50148 [details] [details]) > > Nice. Eventually we need to move more stuff out of the webkitpy root > > directory. > > Thanks, Adam! > > I'll create a master bug for that mini-project so we can discuss how we'd like > to organize things and because the move may be spread over more than one patch. I created the report here: https://bugs.webkit.org/show_bug.cgi?id=36093