[webkit-reviews] review granted: [Bug 64440] garden-o-matic should show test results : [Attachment 100645] Patch

[bugzilla-daemon at webkit.org]

Adam Roben (:aroben) <aroben at apple.com> has granted Adam Barth
<abarth at webkit.org>'s request for review:
Bug 64440: garden-o-matic should show test results
https://bugs.webkit.org/show_bug.cgi?id=64440

Attachment 100645: Patch
https://bugs.webkit.org/attachment.cgi?id=100645&action=review

------- Additional Comments from Adam Roben (:aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=100645&action=review


> Tools/ChangeLog:14
> +	   zoom in on elements of the details pain, and images likely aren't
sized

"pain"? ;-)

> Tools/ChangeLog:25
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/config.js:
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/index.html:
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/main.css:
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/main.js:
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/results.js:
> +	   *
Scripts/webkitpy/tool/servers/data/gardeningserver/results_unittests.js:
> +	   *
Scripts/webkitpy/tool/servers/data/gardeningserver/run-unittests.html:
> +	   * Scripts/webkitpy/tool/servers/data/gardeningserver/ui.js:
> +	   *
Scripts/webkitpy/tool/servers/data/gardeningserver/ui_unittests.js:

It would be nice to have some lower-level explanations of what's going on in
your patch.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/main.js:75
> +	   var children = $(content).children();

I'm not that familiar with jQuery. But is the $() around content really needed
here? You already got content from a jQuery function, so I'd assume it's
already been properly jQuery-ized.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/main.js:81
> +		   $(content).children().hide().fadeIn('fast',
dismissButterbar);

Same question here.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js:20
> -    '-expected.txt',
> -    '-actual.txt',
> +    // '-expected.txt',
> +    // '-actual.txt',

Why did you make this change?