[Webkit-unassigned] [Bug 99246] [TestResultServer] Move the resource loading into a dedicated class

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Ojan Vafai <ojan at chromium.org>  2012-10-13 09:49:50 PST ---
(From update of attachment 168549)
I haven't looked closely at this patch since it's not up for review, but at a high-level this looks great! It would be especially great if you could find a way to write tests for the new class (e.g. by stubbing out the bits that do the actual network requests).

A couple thoughts:
1. Could you wrap all the code in resourceLoader.js in an anonymous function, add a namespace and just export that? See http://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/ui/results.js as an example. Maybe the namespace could be "loader"? 
2. Name the file whatever the namespace name is, so loader.js if we choose "loader" for the namespace for consistency with garden-o-matic's naming style. I'm trying to get the flakiness dashboard to be more consistent in style with garden-o-matic.
3. Also for consistency with garden-o-matic, lets set the whole prototype to a new object at once. See base.RequestTracker http://trac.webkit.org/browser/trunk/Tools/BuildSlaveSupport/build.webkit.org-config/public_html/TestFailures/scripts/base.js#L192 as an example.

Thanks for working on this! I'm very excited to see this code improved.

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