[webkit-reviews] review granted: [Bug 38693] cleanup json_results_generator dependencies so that non-layout-tests can also use it safely : [Attachment 59954] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted 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 59954: Patch
https://bugs.webkit.org/attachment.cgi?id=59954&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
Thanks for following up with this. Please fix the nits below before committing.


WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator
.py:190
 +	    PASS_RESULT, NO_DATA_RESULT and etc) that indicates the test result

nit: Common style for this would be the following (no "and"):
PASS_RESULT, NO_DATA_RESULT, etc)

WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator
.py:451
 +  # Please keep the interface until the other script is cleaned up.
Nit: This should probably be phrased as a FIXME. Something like:
# FIXME: Remove this interface once the other script is cleaned up.

Also, a link pointing to the other script (e.g. in src.chromium.org) wouldn't
hurt.

WebKitTools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator
_unittest.py:42
 +  class JSONGenerationTest(unittest.TestCase):
How about calling this JSONResultsGeneratorTest?