[Webkit-unassigned] [Bug 61066] Double-click on test queued by RebaselineServer to open test in main UI

[bugzilla-daemon at webkit.org]

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





--- Comment #3 from Ojan Vafai <ojan at chromium.org>  2011-07-06 00:26:58 PST ---
(From update of attachment 93925)
View in context: https://bugs.webkit.org/attachment.cgi?id=93925&action=review

A few nits. Otherwise, code seems fine to me. I'm not familiar enough with the surrounding code code to be confident setSelectedTest is doing the right thing. Mihai, mind taking a look?

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/main.js:271
> +    var failureType = test.actual + ' (expected ' + test.expected + ')';

Duplicating this code from line 122 seems fragile to me. Could you create a helper function that takes a test and returns it's failureType?

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/queue.js:47
> +          setSelectedTest(
> +              this.options[this.selectedIndex].value)

Indentation here is off. Also, WebKit has no line-length limit. This would be easier to read as a single line.

> Tools/Scripts/webkitpy/tool/commands/data/rebaselineserver/util.js:60
> +

Perhaps this should throw an error (or at least log a warning to the console) if you pass it a value that's not in the select element?

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