[webkit-reviews] review granted: [Bug 49763] Rebaseline server: add rebaseline queue : [Attachment 74301] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 49763: Rebaseline server: add rebaseline queue
https://bugs.webkit.org/show_bug.cgi?id=49763

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

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=74301&action=review

Just some style nits.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:58
> +    $('toggle-log').addEventListener('click', function() {toggle('log')});

Nit: Spaces in the function body?  You seem to do this in other places.  E.g.,
{ toggle('log'); }

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:48
> +    this._toggleNode.addEventListener('click', function()
{toggle('queue')});

Ditto.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:104

> +    if (this._selectNode.selectedIndex == -1) return;

Nit: Move the return to a new line?

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:118

> +    if (!queueOption) return;

Ditto.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:144

> +    // FIXME: actually rebaseline
> +    log('Rebaselining ' + testName + '...');

I see, this is just the UI.  OK.

> WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js:59
> +function log(text, opt_type)

Nit: optType seems a bit more consistent, although I realize optional arguments
are a bit different.