<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Layout test should generate performance metrics"
   href="https://bugs.webkit.org/show_bug.cgi?id=192030#c26">Comment # 26</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Layout test should generate performance metrics"
   href="https://bugs.webkit.org/show_bug.cgi?id=192030">bug 192030</a>
              from <span class="vcard"><a class="email" href="mailto:dean_johnson@apple.com" title="Dean Johnson <dean_johnson@apple.com>"> <span class="fn">Dean Johnson</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=358550&action=diff" name="attach_358550" title="Patch">attachment 358550</a> <a href="attachment.cgi?id=358550&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=358550&action=review">https://bugs.webkit.org/attachment.cgi?id=358550&action=review</a>

Looks good overall, just a few nits on style / naming.

<span class="quote">> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:122
> +def _add_test_perf_metric(path, time, tests, depth, target_depth):</span >

Talked with Jonathan about target_depth as I was a bit confused by it. Could you rename it to depth_limit, which implies it is okay to exit before hitting target_depth?

<span class="quote">> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:124
> +    Breaks a test name into chunks by directory, and keep track the depth.</span >

the depth -> of the depth.

Both lines may be able to be simplified to:
"Aggregate test time to result for a given test at a specified target_depth."

<span class="quote">> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:144
> +                    }}}</span >

Is there a reason you expanded this block, but not the one below? We should try to be consistent in style here.

<span class="quote">> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:183
> +        # for now, we only sent two level of categories</span >

sent -> send
level -> levels</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>