[Webkit-unassigned] [Bug 60869] Convert json_results_generator.py to output version 4 JSON.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=60869





--- Comment #23 from Ojan Vafai <ojan at chromium.org>  2011-05-23 12:28:51 PST ---
(In reply to comment #17)
> (From update of attachment 94374 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=94374&action=review
> 
> Ojan. :(  You've gone soft.  I don't think you read this very carefully.

Sorry, was a bit rushed. I just focused on correctness.

> > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:86
> > +        if prefix:
> > +            fullname = prefix + "/" + name
> > +        else:
> > +            fullname = name
> 
> I would have written this as a ternary since we use Python 2.5+

Meh. Personally I think python ternaries are less readable than an if/else.

> > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:104
> > +    if not "/" in path:
> 
> I assume we always use / and don't need to use os.path.sep?

Correct. os.path.sep would be wrong.

> > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:134
> > +        add_path_to_trie(test, int(1000 * test_result.test_run_time), trie)
> 
> I wish our time variables would have their unit type in their name.  ms or second or whatever would make these types of multiplctaion much easier to read.

Agreed. Doesn't need to block this patch though, right?

> > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:562
> > +        thisTest = tests
> 
> This is not python style.
>
> > Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator_unittest.py:208
> > +        # FIXME(aboxhall): re-work tests to be more comprehensible and comprehensive.
> 
> We don't generally name FIXME's in WebKit.  Also, all comments in webkit should be sentences, beginning with a capital and ending with period.

Whoops. Sorry. I'm so used to both syntaxes my eyes just glossed over these.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.