Summary: | garden-o-matic's analyzeUnexpectedFailures needs a completion callback. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Component: | Tools / Tests | Assignee: | Dimitri Glazkov (Google) <dglazkov> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 64188 | ||||||||
Attachments: |
|
Description
Dimitri Glazkov (Google)
2011-08-12 14:17:48 PDT
Created attachment 103814 [details]
Patch
Comment on attachment 103814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103814&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/main.js:292 > + model.analyzeUnexpectedFailures(showUnexpectedFailure, function() { > + dismissButterbar(); > + }); You can just pass dismissButterbar as the completion callback. You don't need the thunk. > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/model.js:126 > + var queriesInFlight = Object.keys(unexpectedFailures).length; please use base.RequestTracker. Created attachment 103833 [details]
Patch
Comment on attachment 103833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103833&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js: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.js: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? Comment on attachment 103833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=103833&action=review >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js: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. Yep. I did. There were only two other callsites. >> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:117 >> +base.RequestTracker.prototype = { > > I prefer the other style of modifying the prototype, but whatever. :) Comment on attachment 103833 [details] Patch Clearing flags on attachment: 103833 Committed r93016: <http://trac.webkit.org/changeset/93016> All reviewed patches have been landed. Closing bug. |