Bug 111785

Summary: Dashboard cleanup: Create ui.Errors
Product: WebKit Reporter: Julie Parent <jparent>
Component: Tools / TestsAssignee: Julie Parent <jparent>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Removed some dead code, this is ready for review.
none
Patch for landing none

Description Julie Parent 2013-03-07 15:36:18 PST
Dashboard cleanup: Create ui.Errors
Comment 1 Julie Parent 2013-03-07 15:52:39 PST
Created attachment 192099 [details]
Patch
Comment 2 Julie Parent 2013-03-07 16:58:44 PST
Created attachment 192110 [details]
Removed some dead code, this is ready for review.
Comment 3 Ojan Vafai 2013-03-11 13:55:57 PDT
Comment on attachment 192110 [details]
Removed some dead code, this is ready for review.

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

Just a bunch of style nits.

> Tools/TestResultServer/static-dashboards/ui.js:201
> +    // String of error messages to display to the user.

This comments aren't terribly useful.

> Tools/TestResultServer/static-dashboards/ui.js:202
> +    this._errorMsgs = '';

s/Msgs/Messages

or, even better, this._messages?

> Tools/TestResultServer/static-dashboards/ui.js:204
> +    this._errorElt = null;

s/Elt/Element

or, even better, this._containerElement?

> Tools/TestResultServer/static-dashboards/ui.js:208
> +    // If there are errors, show big and red UI for errors so as to be noticed.

This comment doesn't really add value.

> Tools/TestResultServer/static-dashboards/ui.js:219
> +

Nit: extra newline

> Tools/TestResultServer/static-dashboards/ui.js:222
> +    // Record a new error message.
> +    // @param {string} errorMsg The message to show to the user.

I know this comment was in the old code, but it's pretty useless.

> Tools/TestResultServer/static-dashboards/ui.js:223
> +    addError: function(errorMsg)

s/Msg/Message...or, even better, just "message"
Comment 4 Julie Parent 2013-03-11 14:40:15 PDT
Created attachment 192562 [details]
Patch for landing
Comment 5 Julie Parent 2013-03-11 14:40:29 PDT
Comment on attachment 192110 [details]
Removed some dead code, this is ready for review.

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

>> Tools/TestResultServer/static-dashboards/ui.js:201
>> +    // String of error messages to display to the user.
> 
> This comments aren't terribly useful.

removed

>> Tools/TestResultServer/static-dashboards/ui.js:202
>> +    this._errorMsgs = '';
> 
> s/Msgs/Messages
> 
> or, even better, this._messages?

done

>> Tools/TestResultServer/static-dashboards/ui.js:204
>> +    this._errorElt = null;
> 
> s/Elt/Element
> 
> or, even better, this._containerElement?

done

>> Tools/TestResultServer/static-dashboards/ui.js:208
>> +    // If there are errors, show big and red UI for errors so as to be noticed.
> 
> This comment doesn't really add value.

removed

>> Tools/TestResultServer/static-dashboards/ui.js:219
>> +
> 
> Nit: extra newline

removed

>> Tools/TestResultServer/static-dashboards/ui.js:222
>> +    // @param {string} errorMsg The message to show to the user.
> 
> I know this comment was in the old code, but it's pretty useless.

removed

>> Tools/TestResultServer/static-dashboards/ui.js:223
>> +    addError: function(errorMsg)
> 
> s/Msg/Message...or, even better, just "message"

done
Comment 6 WebKit Review Bot 2013-03-11 18:09:21 PDT
Comment on attachment 192562 [details]
Patch for landing

Clearing flags on attachment: 192562

Committed r145447: <http://trac.webkit.org/changeset/145447>
Comment 7 WebKit Review Bot 2013-03-11 18:09:25 PDT
All reviewed patches have been landed.  Closing bug.