Bug 111785 – Dashboard cleanup: Create ui.Errors

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Julie Parent]

Dashboard cleanup: Create ui.Errors

[Julie Parent]

Created attachment 192099 [details]
Patch

[Julie Parent]

Created attachment 192110 [details]
Removed some dead code, this is ready for review.

[Ojan Vafai]

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"

[Julie Parent]

Created attachment 192562 [details]
Patch for landing

[Julie Parent]

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

[WebKit Review Bot]

Comment on attachment 192562 [details]
Patch for landing

Clearing flags on attachment: 192562

Committed r145447: <http://trac.webkit.org/changeset/145447>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.