Summary: | Dashboard cleanup: Create ui.Errors | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Julie Parent <jparent> | ||||||||
Component: | Tools / Tests | Assignee: | 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
Julie Parent
2013-03-07 15:36:18 PST
Created attachment 192099 [details]
Patch
Created attachment 192110 [details]
Removed some dead code, this is ready for review.
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" Created attachment 192562 [details]
Patch for landing
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 on attachment 192562 [details] Patch for landing Clearing flags on attachment: 192562 Committed r145447: <http://trac.webkit.org/changeset/145447> All reviewed patches have been landed. Closing bug. |