Bug 49763 – Rebaseline server: add rebaseline queue

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mihai Parparita]

Rebaseline server: add rebaseline queue

[Mihai Parparita]

Created attachment 74301 [details]
Patch

[Tony Chang]

Comment on attachment 74301 [details]
Patch

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.

[Mihai Parparita]

(In reply to comment #2)
> > 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'); }

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

Done.

> > WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:104
> > +    if (this._selectNode.selectedIndex == -1) return;
> 
> Nit: Move the return to a new line?

Done.

> > WebKitTools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:118
> > +    if (!queueOption) return;
> 
> Ditto.

Done

> > 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.

I just dropped the opt_ altogether, in the absence of the JS compiler there isn't really a need to signal that an argument is optional.

[Mihai Parparita]

Committed r72364: <http://trac.webkit.org/changeset/72364>