[webkit-reviews] review granted: [Bug 111785] Dashboard cleanup: Create ui.Errors : [Attachment 192110] Removed some dead code, this is ready for review.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Mar 11 13:53:31 PDT 2013


Ojan Vafai <ojan at chromium.org> has granted Julie Parent <jparent at google.com>'s
request for review:
Bug 111785: Dashboard cleanup: Create ui.Errors
https://bugs.webkit.org/show_bug.cgi?id=111785

Attachment 192110: Removed some dead code, this is ready for review.
https://bugs.webkit.org/attachment.cgi?id=192110&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
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"


More information about the webkit-reviews mailing list