Bug 66245 - garden-o-matic frontend needs a generic way to track updates.
Summary: garden-o-matic frontend needs a generic way to track updates.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 66144
  Show dependency treegraph
 
Reported: 2011-08-15 12:20 PDT by Dimitri Glazkov (Google)
Modified: 2011-08-15 14:31 PDT (History)
1 user (show)

See Also:


Attachments
Patch (5.40 KB, patch)
2011-08-15 12:22 PDT, Dimitri Glazkov (Google)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2011-08-15 12:20:16 PDT
garden-o-matic frontend needs a generic way to track updates.
Comment 1 Dimitri Glazkov (Google) 2011-08-15 12:22:57 PDT
Created attachment 103941 [details]
Patch
Comment 2 Dimitri Glazkov (Google) 2011-08-15 12:23:23 PDT
As mentioned in https://bugs.webkit.org/show_bug.cgi?id=66144#c8.
Comment 3 Adam Barth 2011-08-15 13:24:49 PDT
Comment on attachment 103941 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103941&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:271
> +        thisObject = thisObject || {};

You and MarkM have been talking!

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:274
> +            callback.call(thisObject || item, item, key, !!this._updated[key]);

How can thisObject be falsy here?  It was falsy, it would be replaced with {} above.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:278
> +        removeCallback = removeCallback || function() {}

Missing ;

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:280
> +            if (!updated) {

I'd use early return.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:281
> +                removeCallback.call(thisObject || item, item);

I'd make this match forEach, whichever way you decided to make forEach work.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:279
> +    dict.purge(function() {
> +        removeCount++;
> +    });

I'd add one test with purge that uses the thisObject parameter.
Comment 4 Dimitri Glazkov (Google) 2011-08-15 14:27:46 PDT
Comment on attachment 103941 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103941&action=review

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:274
>> +            callback.call(thisObject || item, item, key, !!this._updated[key]);
> 
> How can thisObject be falsy here?  It was falsy, it would be replaced with {} above.

Oops. I didn't mean to leave the one at the top.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:278
>> +        removeCallback = removeCallback || function() {}
> 
> Missing ;

I blame Python.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:280
>> +            if (!updated) {
> 
> I'd use early return.

Sounds good, will change.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base.js:281
>> +                removeCallback.call(thisObject || item, item);
> 
> I'd make this match forEach, whichever way you decided to make forEach work.

ok.

>> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/base_unittests.js:279
>> +    });
> 
> I'd add one test with purge that uses the thisObject parameter.

Will do.
Comment 5 Dimitri Glazkov (Google) 2011-08-15 14:31:49 PDT
Committed r93061: <http://trac.webkit.org/changeset/93061>