[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