[webkit-reviews] review denied: [Bug 38693] cleanup json_results_generator dependencies so that non-layout-tests can also use it safely : [Attachment 58105] Take 2.1 (style fix)

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Kinuko Yasuda <kinuko at chromium.org>'s
request for review:
Bug 38693: cleanup json_results_generator dependencies so that non-layout-tests
can also use it safely
https://bugs.webkit.org/show_bug.cgi?id=38693

Attachment 58105: Take 2.1 (style fix)
https://bugs.webkit.org/attachment.cgi?id=58105&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
Very sorry this took so long to review. Now that I can see it in rietveld it's
much easier to see that this is mostly just moving code around. Anyways, r- due
to a few minor issues. Otherwise this looks good.
---------------------------------
http://wkrietveld.appspot.com/38693/diff/1/2
File WebKitTools/ChangeLog (right):

http://wkrietveld.appspot.com/38693/diff/1/2#newcode18
WebKitTools/ChangeLog:18: * Scripts/webkitpy/test_package/__init__.py: Added.
I think a better home for these files would be a new directory in "common". For
example:
webkitpy/common/test.

http://wkrietveld.appspot.com/38693/diff/1/5
File
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py
(right):

http://wkrietveld.appspot.com/38693/diff/1/5#newcode46
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py:4
6: # Note: They need to match with the webkitpy.test_package.test_results.
I think you can do this in the import statement and avoid duplicating this
list. Namely:
from webkitpy.test_package.test_results import *

If that doesn't work, I'd rather we just use test_results.PASS instead of PASS
in this file.