[Webkit-unassigned] [Bug 136386] Update webkit dashboard to support performance bots

[bugzilla-daemon at webkit.org]

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


Alexey Proskuryakov <ap at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #237383|review+                     |review-
               Flag|                            |




--- Comment #4 from Alexey Proskuryakov <ap at webkit.org>  2014-08-29 16:55:48 PST ---
(From update of attachment 237383)
View in context: https://bugs.webkit.org/attachment.cgi?id=237383&action=review

Dana and myself checked whether this new column fits on a 13" screen, and it doesn't quite. Fortunately, the layout can be squeezed very very slightly to make it fit, but we can't have any more columns (which is a bummer, as there are also leaks bots and other misc ones).

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Buildbot.js:62
> +    Performance: "perf"

We avoid abbreviations in WebKit code base. Please change all "perf"s into complete words.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:128
> +                if (queue.perfType) {
> +                    label = queue.perfType;
> +                }

There are no braces around single line blocks in WebKit coding style.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/BuildbotTesterQueueView.js:146
> +        if (this.perfQueues) {
> +            appendBuild.call(this, this.perfQueues);
> +        }

Ditto. Also, this check is always true anyway, as [] converts to true.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/Main.js:56
>          var buildType = queue.debug ? "debug" : "release";
> +        buildType = queue.perfType ? "perfType" : buildType;

I'd probably use a nested conditional here - it's unnecessarily confusing to initialize a variable and immediately override it. Not a big deal though.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/WebKitBuildbot.js:54
>> +        "EFL Linux 64-bit Release WK2 (Perf)": {platform: Dashboard.Platform.LinuxEFL, tester: true, testCategory: Buildbot.TestCategory.Performance}
> 
> These seem unrelated to the patch (although I suppose it's okay to add them now while you're in the neighborhood).

OK to land, but I'd still suggest splitting this into a separate patch.

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