[webkit-reviews] review granted: [Bug 66166] garden-o-matic's analyzeUnexpectedFailures needs a completion callback. : [Attachment 103833] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 66166: garden-o-matic's analyzeUnexpectedFailures needs a completion
callback.
https://bugs.webkit.org/show_bug.cgi?id=66166

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103833&action=review


>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.j
s:114
> +    this._tryCallback();

Did you check the other call sites to see what they thought of this new
behavior?  I seem to remember a bunch of them doing this manually.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.j
s:117
> +base.RequestTracker.prototype = {

I prefer the other style of modifying the prototype, but whatever.

>
Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/model.
js:126
> +    var tracker = new
base.RequestTracker(Object.keys(unexpectedFailures).length,
completionCallback);

Why Object.keys and not base.keys ?  Should base.keys == Object.keys when
Object.keys is available?