Bug 106356 - Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
Summary: Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
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: Julie Parent
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-08 11:32 PST by Julie Parent
Modified: 2013-01-08 12:34 PST (History)
5 users (show)

See Also:


Attachments
Patch (8.44 KB, patch)
2013-01-08 11:37 PST, Julie Parent
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julie Parent 2013-01-08 11:32:55 PST
Dashboard cleanup: Remove globals g_buildersThatFailedToLoad and g_staleBuilders
Comment 1 Julie Parent 2013-01-08 11:37:03 PST
Created attachment 181716 [details]
Patch
Comment 2 Dirk Pranke 2013-01-08 11:54:29 PST
Comment on attachment 181716 [details]
Patch

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

> Tools/TestResultServer/static-dashboards/loader.js:81
> +            resourceLoadingComplete(this._getLoadingErrorMessages());

I probably would've just inlined _getLoadingErrorMessages() here; it looks like this is the only caller, right?
Comment 3 Julie Parent 2013-01-08 11:56:58 PST
Comment on attachment 181716 [details]
Patch

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

>> Tools/TestResultServer/static-dashboards/loader.js:81
>> +            resourceLoadingComplete(this._getLoadingErrorMessages());
> 
> I probably would've just inlined _getLoadingErrorMessages() here; it looks like this is the only caller, right?

Yeah, it is the only caller.  It seemed cleaner to me to have it separate, particularly as I was imagining a future world where we might have more robust or numerous errors, and having all of that logic dumped inside of here would clog it up.  Happy to make that change if you prefer though.
Comment 4 Dirk Pranke 2013-01-08 12:12:16 PST
it's up to you.
Comment 5 WebKit Review Bot 2013-01-08 12:34:30 PST
Comment on attachment 181716 [details]
Patch

Clearing flags on attachment: 181716

Committed r139097: <http://trac.webkit.org/changeset/139097>
Comment 6 WebKit Review Bot 2013-01-08 12:34:33 PST
All reviewed patches have been landed.  Closing bug.