[webkit-reviews] review granted: [Bug 64446] garden-o-matic should have a "rebaseline" button : [Attachment 100813] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 64446: garden-o-matic should have a "rebaseline" button
https://bugs.webkit.org/show_bug.cgi?id=64446

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100813&action=review


> Tools/ChangeLog:17
> +	   2) The baselines are not optimized for redundancy (meaning you can
have
> +	      identical baselines in both chromium-mac and chromium-win).

Koz is a checkin or two away from having a tool that does this. Then this could
use that? We should see if he'd be willing to prioritize getting this done.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/index.html:23
> +  <div class="toolbar">
> +    <div class="actions">
> +	 <a class="rebaseline" href="#">Rebaseline</a>
> +	 <a class="dismiss" href="#">Close</a>
> +    </div>
> +    <div class="status"></div>
> +  </div>
> +  <div class="content"></div>

indentation is off here. also, shouldn't these be 4 space indents? i realize we
don't have a firm style here, but may as well be consistent on indentation
size.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js:126
> +	   return ['png'];
> +    case TEXT:
> +	   return ['txt'];
> +    case IMAGE_TEXT:
> +	   return ['txt', 'png'];

These strings should be constants IMO.

> Tools/Scripts/webkitpy/tool/servers/data/gardeningserver/results.js:144
> +    for (var i= 0; i < failureTypeList.length; ++i) {
> +	   if (results.failureTypeToExtensionList(failureTypeList[i]).length >
0)
> +	       return true;
> +    }
> +    return false;

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/some


return failureTypeList.some(function(element) {
    return results.failureTypeToExtensionList(failureTypeList[i]).length > 0;
});