[Webkit-unassigned] [Bug 117941] code coverage report

[bugzilla-daemon at webkit.org]

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


Joseph Pecoraro <joepeck at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #205428|1                           |0
        is obsolete|                            |




--- Comment #20 from Joseph Pecoraro <joepeck at webkit.org>  2013-06-26 12:07:09 PST ---
(From update of attachment 205428)
View in context: https://bugs.webkit.org/attachment.cgi?id=205428&action=review

Looks good to me

> Tools/CodeCoverage/results-template.html:8
> +                padding: 0px;

Suggestion: Everywhere you have 0, you can drop the units "px", "%", etc. But I'll leave that decision up to you.

> Tools/CodeCoverage/results-template.html:159
> +                xhr.onreadystatechange=function()

Style: '=' => ' = '

> Tools/CodeCoverage/results-template.html:161
> +                    if (xhr.readyState === 4) {

Suggestion: 4 => XMLHttpRequest.DONE

> Tools/CodeCoverage/results-template.html:197
> +                li.className='file';

Style: '=' => ' = '

> Tools/CodeCoverage/results-template.html:234
> +                    children.style.display='';
> +                } else {
> +                    img.src = rightArrow;
> +                    children.style.display='none';

Style: '=' => ' = '

> Tools/CodeCoverage/results-template.html:356
> +        <script id="json" type="application/json">%PeformanceTestsResultsJSON%</script>

Style: This is the only place where you use double quotes intend of single quotes. I'd change it for consistency.

> Tools/Scripts/generate-coverage-data:78
> +$templateHtml =~ s/%PeformanceTestsResultsJSON%/$jsonData/g;

The /g shouldn't be necessary. There will probably only ever be one %PerformanceTestsResultsJSON%, right? Its like a 9MB string.

> Tools/Scripts/generate-coverage-data:85
> +system "open $url";

I'd recommend quoting the path in case anyone has spaces in their path leading up to WebKitBuild.

    system "open '$url'";

Of course using perl exec APIs there is probably a safer way that handles this. Probably:

    system "open", $url;

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